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

Bugfix FXIOS-10509 ⁃ [Experiment] The Menu Redesign CFR should be displayed for new users that have seen the old menu #23090

Conversation

dicarobinho
Copy link
Collaborator

πŸ“œ Tickets

Jira ticket
Github issue

πŸ’‘ Description

Display the Menu CFR for fresh installs but only fot the users that have seen before the old menu

πŸ“ Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • Updated the PR name to follow our PR naming guidelines
  • Wrote unit tests and/or ensured the tests suite is passing
  • When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • If needed, I updated documentation / comments for complex code and public methods
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

…played for new users that have seen the old menu
@dicarobinho dicarobinho marked this pull request as ready for review November 13, 2024 15:28
@dicarobinho dicarobinho requested a review from a team as a code owner November 13, 2024 15:28
@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Nov 13, 2024

Messages
πŸ“– Project coverage: 32.25%
πŸ“– Edited 3 files
πŸ“– Created 0 files

Client.app: Coverage: 30.15

File Coverage
BrowserViewController.swift 4.67% ⚠️
MainMenuViewController.swift 21.43% ⚠️

Generated by 🚫 Danger Swift against 3d6bb7e

if InstallType.get() == .fresh {
if let photonMainMenuShown = profile.prefs.boolForKey(PrefsKeys.PhotonMainMenuShown),
photonMainMenuShown {
return viewProvider.shouldPresentContextualHint() ? true : false
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused here

                return viewProvider.shouldPresentContextualHint() ? true : false

if true return true and if false return false? Why are we doing this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is true... changed it! Thank you!

@adudenamedruby adudenamedruby merged commit a8321d3 into main Nov 13, 2024
9 of 10 checks passed
@adudenamedruby adudenamedruby deleted the afarcasanu/fxios_10509_23042_menu_redesign_cfr_should_be_displayed_for_new_users_that_have_seen_the_old_menu branch November 13, 2024 20:23
@adudenamedruby
Copy link
Contributor

@Mergifyio backport release/v133

Copy link
Contributor

mergify bot commented Nov 13, 2024

backport release/v133

βœ… Backports have been created

mergify bot pushed a commit that referenced this pull request Nov 13, 2024
…users that have seen the old menu (#23090)

* FXIOS-10509 #23042 ⁃ [Experiment] The Menu Redesign CFR should be displayed for new users that have seen the old menu

* Refactored two lines of code

(cherry picked from commit a8321d3)
dicarobinho added a commit that referenced this pull request Nov 14, 2024
…played for new users that have seen the old menu (backport #23090) (#23105)

Bugfix FXIOS-10509 [Menu] [CFR] Menu CFR should be displayed for new users that have seen the old menu (#23090)

* FXIOS-10509 #23042 ⁃ [Experiment] The Menu Redesign CFR should be displayed for new users that have seen the old menu

* Refactored two lines of code

(cherry picked from commit a8321d3)

Co-authored-by: dicarobinho <61138287+dicarobinho@users.noreply.github.com>
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.

3 participants