Skip to content

Commit

Permalink
Fix 3/4 sun issue (#608)
Browse files Browse the repository at this point in the history
* Fix 3/4 sun issue

* fix typo

* Fix import path

* Remove empty config files
  • Loading branch information
dgarciabriseno authored Sep 10, 2024
1 parent 7280e70 commit b45156b
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 11 deletions.
8 changes: 0 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,6 @@ jobs:
echo $(ls)
- name: Setup environment file
run: mv compose/.env.example .env
- name: Make writeable configuration files
run: |
touch api/install/settings/settings.cfg
chmod o+rw api/install/settings/settings.cfg
touch api/settings/Config.ini
chmod o+rw api/settings/Config.ini
touch api/settings/Config.php
chmod o+rw api/settings/Config.php
- name: Start local Helioviewer environment
id: docker
run: |
Expand Down
3 changes: 2 additions & 1 deletion resources/js/Tiling/Layer/TileLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ var TileLayer = Layer.extend(
/**
*
*/
updateImageScale: function (scale, tileVisibilityRange) {
updateImageScale: function (scale) {
let tileVisibilityRange = this.tileVisibilityRange;
this.viewportScale = scale;

// The general visibility range doesn't account for any x/y offsets.
Expand Down
3 changes: 1 addition & 2 deletions resources/js/Tiling/Manager/TileLayerManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,9 @@ var TileLayerManager = LayerManager.extend(
}

this.viewportScale = scale;
var self = this;

$.each(this._layers, function () {
this.updateImageScale(scale, self.tileVisibilityRange);
this.updateImageScale(scale);
});
},

Expand Down
39 changes: 39 additions & 0 deletions tests/mobile/regression/tiles.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { test, expect } from '@playwright/test';
import { HvMobile } from '../../page_objects/mobile_hv';

/**
* A recurring issue in Helioviewer deals with computing which tiles should
* be displayed in the viewport based on the screen size, zoom amount, and
* the image container position. This test verifies that tiles are loaded
* properly when the viewport region intersects with tile boundaries.
*
* This test was written for a case where 3/4 of the sun does not show after
* zoomg in.
* This test was made for: https://github.com/Helioviewer-Project/helioviewer.org/issues/607
* This issue references the problem on a desktop view, I was able to reproduce
* only on a mobile view, but the viewport code is shared, so hopefully fixing
* this problem fixes both problems.
*
* Test Steps:
* 1. Drag the sun up, the issue only happens with the sun in certain positions.
* 2. Zoom In enough to trigger the resolution update
* 3. Verify that all 4 expected image tiles are loaded.
*
* This test verifies that the black space does NOT remain, and that the tile does get loaded
* when it is dragged into the viewport.
*/
test(`[Mobile] Verify image tiles are loaded when the viewport pans to tile boundaries after zooming in and out`, async ({ page }) => {
let hv = new HvMobile(page);
await hv.Load();
await hv.WaitForLoad();
// 1. Drag the sun up
await hv.moveViewport(0, -150);
// 2. Zoom in one step to trigger the resolution update
await hv.ZoomIn(1);
await hv.WaitForLoad();
// 3. At this level, 4 tiles should be visible
expect(page.locator("//img[contains(@src, 'x=0&y=0')]")).toHaveCount(1);
expect(page.locator("//img[contains(@src, 'x=-1&y=0')]")).toHaveCount(1);
expect(page.locator("//img[contains(@src, 'x=0&y=-1')]")).toHaveCount(1);
expect(page.locator("//img[contains(@src, 'x=-1&y=-1')]")).toHaveCount(1);
});
20 changes: 20 additions & 0 deletions tests/page_objects/mobile_hv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,13 @@ class HvMobile {
await this.page.locator('#timeNowBtn_mob_td #timeNowBtn').click();
}

async ZoomIn(steps: number) {
for (let i = 0; i < steps; i++) {
await this.page.keyboard.press("+");
await this.page.waitForTimeout(250);
}
}

async ZoomOut(steps: number) {
for (let i = 0; i < steps; i++) {
await this.page.keyboard.press("-");
Expand All @@ -220,6 +227,19 @@ class HvMobile {
await expect(this.page.getByText("No shared movies found.")).toHaveCount(1);
await this.CloseYoutubeVideosDialog();
}

/**
* Move the viewport by the given amount
* @param x Horizontal amount
* @param y Vertical amount
*/
async moveViewport(x: number, y: number) {
const INITIAL_POSITION = { x: 150, y: 400 };
await this.page.mouse.move(INITIAL_POSITION.x, INITIAL_POSITION.y);
await this.page.mouse.down();
await this.page.mouse.move(INITIAL_POSITION.x + x, INITIAL_POSITION.y + y);
await this.page.mouse.up();
}
}

export { HvMobile }

0 comments on commit b45156b

Please sign in to comment.