Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

web: don't display the account menu item when offline snapshots are enabled #5799

Merged
merged 1 commit into from
May 11, 2022

Conversation

lizzthabet
Copy link
Contributor

@lizzthabet lizzthabet commented May 10, 2022

This PR hides the account menu item and dialog when offline snapshot creation is enabled. Since people won't need an account to create an offline snapshot, account information is no longer relevant.

Closes ch13514

@lizzthabet lizzthabet self-assigned this May 10, 2022
@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #13514: Hide accounts menu item with offline snapshots.


const hideAccountDialog = features.isEnabled(Flag.OfflineSnapshotCreation)
if (hideAccountDialog) {
return null
Copy link
Member

Choose a reason for hiding this comment

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

this might be a silly question...but shouldn't it be impossible to hit this codepath? because disabling the dialog happens further upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not a silly question! i'm being extra-defensive in this code path and mostly added this case due to my local testing. you can open the account dialog, enable the feature flag, and the dialog will stick around even though the menu item is gone.

@lizzthabet lizzthabet merged commit 9a6593a into master May 11, 2022
@lizzthabet lizzthabet deleted the lizz/chore/hide-account-icon-snapshots branch May 11, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants