Skip to content

Conversation

@gribnoysup
Copy link
Collaborator

Spotted this issue while debugging an e2e test flake: on a fresh compass install guide cues are awkwardly disaplyed over the welcome modal. Interacting with the modal dismisses both, so you don't know anymore where it was pointing. This patch addresses the issue by disabling guide cues in the app completely until welcome modal is hidden.

Before After
Kapture.2025-10-31.at.18.38.41.mp4
Kapture.2025-10-31.at.18.28.44.mp4

The gist of the changes in this patch:

  • Change the logic around showedNetworkOptIn: only set it to true after the modal is closed.
    • Refactor WelcomeModal into a plugin so that all this logic of changing the preference on close can be encapsulated in a single place
    • As a related drive-by: remove ensureDefaultConfigurableUserPreferences from preferences interface and implementation, I couldn't see any reason to have this logic in compass, it just immediately sets some values to slightly different defaults on app start
  • Change GuideCue component to disable itself only visually, but keep the state around so that when re-enabled the items will show up on the screen

@gribnoysup gribnoysup requested a review from a team as a code owner October 31, 2025 17:49
@gribnoysup gribnoysup requested a review from ivandevp October 31, 2025 17:49
@gribnoysup gribnoysup added the fix label Oct 31, 2025
@gribnoysup gribnoysup added the no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) label Nov 3, 2025
Copy link
Collaborator

@ivandevp ivandevp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gribnoysup gribnoysup merged commit c4cdef8 into main Nov 3, 2025
62 of 63 checks passed
@gribnoysup gribnoysup deleted the no-guide-cues-until-welcome-modal-handled branch November 3, 2025 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants