Repository navigation
fix(settings): align guided tour highlights with dialog rows - #2369
Open
jonathanwxh-cell wants to merge 1 commit into
Open
jonathanwxh-cell wants to merge 1 commit into
jonathanwxh-cell wants to merge 1 commit into
Conversation
7 tasks
jonathanwxh-cell
marked this pull request as ready for review
October 16, 2026 04:17
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2364.
The Settings tour can highlight the app bar instead of the setting it describes. On the original code, the Language target was at y=229 while its spotlight started at y=4. Joyride measures offsets differently for targets inside a fixed, scrolling dialog; the inner target div was positioned relative to its row/list rather than the dialog.
Move each target ID onto the complete setting row and keep the Settings lists out of the offset-parent chain. Preserve the dialog scroll container and replace the 500px scroll offset with 20px. Start the tour after the dialog's entrance transition finishes so a fresh visit cannot calculate scrolling from a moving target.
Add browser regression coverage for all seven setting targets in English, Spanish, and Arabic/RTL, in light and dark themes. The checks compare spotlight/row geometry, require the whole target and tooltip to be visible, check the tour images, and verify completion/dismissal persistence and help-button restart.
PLAYWRIGHT_BASE_URLallows testing the local branch instead of the default QA deployment.Validation on Node 22.23.2:
git diff --check; the commit hook also passed.For a local browser run, start the app on port 3100, then run:
PLAYWRIGHT_BASE_URL=http://127.0.0.1:3100 npx playwright test tests/unlogged/settings/tour.spec.js --project=chromium --project=mobile --workers=1Other-tour checks: stepped through the root-board, unlocked-board, communicator-builder, and symbol-search tours. The Communicator spotlight problem also reproduces on the public production app and is reported separately in #2368. I found no Joyride tour in the current Analytics implementation.
Limits: Firefox was attempted but its downloaded executable cannot start on this Windows machine (side-by-side configuration error). Packaged Android/Cordova and physical mobile devices were not tested. Web tour images loaded in all tested language/theme combinations.
Asked about existing ownership and the preferred approach in #2364 before preparing the patch. Ready for maintainer review; additional Firefox and packaged Android verification would be welcome.