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.

Sign up for GitHub

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

Jump to bottom

fix(Mobile): Add a correct check to show or not keybinds section #899

Merged
merged 9 commits into from
Dec 4, 2024
Merged

fix(Mobile): Add a correct check to show or not keybinds section #899

merged 9 commits into from
Dec 4, 2024

Conversation

lgmarchi
Copy link
Collaborator

What this PR does 📖

  • Old code was using width to check if it is mobile or not, but even on web, if user is using with small width, it should keep showing keybinds section, so this PR fix that, and hide this section just in mobile.

Which issue(s) this PR fixes 🔨

  • Resolve #

Special notes for reviewers 🗒️

Additional comments 🎤

@lgmarchi lgmarchi self-assigned this Nov 29, 2024
@github-actions github-actions bot added the Missing dev review Two dev reviews are required on PR label Nov 29, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 29, 2024 •
edited
Loading

Download the app installers for this pull request:

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 29, 2024 •
edited
Loading

Automated tests execution is complete! You can find the Playwright test report here and the Allure Test Report here

@luisecm
Copy link
Contributor

luisecm commented Dec 2, 2024

@lgmarchi Hello Lucas, I noticed that also in this PR the install banner is not shown on web, and this is why automated tests are failing. I noticed that when we mess with the OnMount method we can alter the behavior of the banner. Could you please fix this?

image

@phillsatellite phillsatellite added the QA Requested Changes Changes need to be addressed, something is not working as expected. label Dec 2, 2024
stavares843
stavares843 requested changes Dec 2, 2024
View reviewed changes
Copy link
Member

@stavares843 stavares843 left a comment

Choose a reason for hiding this comment

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

luis comment needs fixing

@stavares843
Copy link
Member

we will retest with the latest PR that was merged

@stavares843 stavares843 removed the QA Requested Changes Changes need to be addressed, something is not working as expected. label Dec 2, 2024
@github-actions github-actions bot added the Missing fixing conflict Needs to fix conflicts label Dec 3, 2024
@luisecm luisecm added the QA Requested Changes Changes need to be addressed, something is not working as expected. label Dec 3, 2024
@stavares843 stavares843 removed Missing fixing conflict Needs to fix conflicts QA Requested Changes Changes need to be addressed, something is not working as expected. labels Dec 3, 2024
@github-actions github-actions bot added the Missing fixing conflict Needs to fix conflicts label Dec 3, 2024
@stavares843 stavares843 removed the Missing fixing conflict Needs to fix conflicts label Dec 3, 2024
@phillsatellite phillsatellite added the QA Approved PR has been tested by QA Team label Dec 4, 2024
@stavares843 stavares843 merged commit f012a05 into dev Dec 4, 2024
11 checks passed
@stavares843 stavares843 deleted the hide-keybinds-for-mobile branch December 4, 2024 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phillsatellite phillsatellite phillsatellite approved these changes

@stavares843 stavares843 stavares843 approved these changes

Assignees

@lgmarchi lgmarchi

Labels
Missing dev review Two dev reviews are required on PR QA Approved PR has been tested by QA Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lgmarchi @luisecm @stavares843 @phillsatellite