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

[joy-ui][button] Fix sticky hover media query issue on mobile #37467

Merged
merged 10 commits into from
Oct 30, 2023
Merged

[joy-ui][button] Fix sticky hover media query issue on mobile #37467

merged 10 commits into from
Oct 30, 2023

Conversation

gitstart
Copy link
Contributor

@gitstart gitstart commented Jun 1, 2023 •
edited by oliviertassinari

Resolves sticky hover effect on mobile devices

Iterate on #36583

This code was written and reviewed by GitStart Community. Growing great engineers, one PR at a time.

@gitstart @seunexplicit
fix: resolve sticky hover effect on buttons on mobile devices
94a194d 
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@mui-bot
Copy link

mui-bot commented Jun 1, 2023 •
edited

Netlify deploy preview

https://deploy-preview-37467--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 4ec2e22

@danilo-leal danilo-leal changed the title MUI-36583B - [base][button] hover media query issue on mobile [base][button] Fix sticky hover media query issue on mobile Jun 1, 2023
@danilo-leal danilo-leal added the package: base-ui Specific to @mui/base label Jun 1, 2023
@gitstart gitstart marked this pull request as ready for review June 7, 2023 16:50
@zannager zannager requested a review from siriwatknp June 8, 2023 08:00
gitstart and others added 2 commits June 9, 2023 02:33
@gitstart
Merge commit '0d3aba1c9433dc5dd79dd9af6821c25b48d6f4f7' of https://gi…
d6c7066 
…thub.com/GitStartHQ/material-ui into MUI-36583B
@gitstart @seunexplicit
chore: format code changes
c4d0971 
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 20, 2023
@gitstart @seunexplicit
fix: resolve merge conflicts
7ec7d5e 
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 27, 2023
@gitstart
Copy link
Contributor Author

@siriwatknp Gentle reminder about this PR

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 29, 2023
@gitstart @seunexplicit
update branch
c7ebd52 
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 9, 2023
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 13, 2023 •
edited

On issues like this, I think that the best course of action is, one component at a time:

  1. Iterate on the Joy UI component. For example here fixing the bug in [base-ui][joy-ui] Hover styles persist after clicking on an element on mobile #36583 and handling https://www.notion.so/mui-org/Olivier-design-review-on-Joy-Design-3ada9a7bcfa44b9fab1fe5032dfb33bb. Once we feel great about it, or that we have done enough move to the next step.
  2. Trash the Base UI demo implementation, copy the Joy UI one, but keep it free of all the overhead that comes with allowing developers to customize the components without owning the source.
  3. As new bugs are found in Base UI or Joy UI, copy the fixes in both places, the look and feel should be identical.

cc @mui/joy-ui

In this case, I feel like fixing Base UI demos that are meant to be reference implementation is wasted time since we will trash all the code to redo it based on Joy UI. So we could focus this PR on Joy UI.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work package: joy-ui Specific to @mui/joy and removed package: base-ui Specific to @mui/base labels Aug 13, 2023
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 15, 2023
@lithdew
Copy link

lithdew commented Sep 5, 2023

Might there be any updates on this? I'd like to make sure hover effects on mobile are properly disabled for an app I'm working on. Happy to also assist with breaking this PR up into smaller PR's.

@gitstart @seunexplicit
implement fix on only Joy UI
b927019 
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 22, 2023
@gitstart
Copy link
Contributor Author

@oliviertassinari we have done the implementation on just the Joy UI library for your review, apologies for the delay. cc @lithdew

@oliviertassinari oliviertassinari changed the title [base][button] Fix sticky hover media query issue on mobile [joy-ui][button] Fix sticky hover media query issue on mobile Sep 24, 2023
siriwatknp
siriwatknp approved these changes Oct 18, 2023
View reviewed changes
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Thanks for the fix and sorry for the delay!

siriwatknp
siriwatknp approved these changes Oct 18, 2023
View reviewed changes
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Thanks for the fix and sorry for the delay!

@siriwatknp
Copy link
Member

@gitstart Can you pull the latest master? I cannot push to your branch.

image

@gitstart
Copy link
Contributor Author

@gitstart Can you pull the latest master? I cannot push to your branch.

image

The branch has been updated, thanks.

@mnajdova
Copy link
Member

cc @siriwatknp anything else blocking this one from being merged?

@siriwatknp siriwatknp merged commit a2fdd9c into mui:master Oct 30, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@siriwatknp siriwatknp siriwatknp approved these changes

@oliviertassinari oliviertassinari Awaiting requested review from oliviertassinari

Assignees
No one assigned
Labels
bug 🐛 Something doesn't work package: joy-ui Specific to @mui/joy
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@gitstart @mui-bot @oliviertassinari @lithdew @siriwatknp @mnajdova @danilo-leal