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

static/Modal.jsx refactor: migrate from Bootstrap to native dialog element adapt to mB redesign #1758

Merged
merged 1 commit into from
Jan 30, 2025

Conversation

hom3mad3
Copy link
Contributor

@hom3mad3 hom3mad3 commented Jan 22, 2025

Replace Bootstrap modals with native HTML element and update styles for the mB redesign.

This change simplifies our modal implementation while improving the mobile experience. The element provides better accessibility and native support for modal behaviors.

styles: mB aplus

Screen.Recording.2025-01-21.at.19.46.40.mov
mobile-sheet.mov

TODO:

  • a-plus dependency

@hom3mad3
Copy link
Contributor Author

hom3mad3 commented Jan 22, 2025

@CarolingerSeilchenspringer @mcastro-lqd Would it be alright to keep the mobile design as suggested above? The full width and radius give it a more native feel, which could create a smoother experience.

@CarolingerSeilchenspringer

@mcastro-lqd can you have a look?

Copy link
Collaborator

@vellip vellip left a comment

Choose a reason for hiding this comment

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

Very good work, looks nice. It was interesting to see the dialog element being used. Haven't done that before. I left a some questions for you, but it looks good already.

adhocracy4/static/Modal.jsx Outdated Show resolved Hide resolved
adhocracy4/static/Modal.jsx Outdated Show resolved Hide resolved
adhocracy4/static/Modal.jsx Outdated Show resolved Hide resolved
adhocracy4/static/Modal.jsx Outdated Show resolved Hide resolved
adhocracy4/static/Modal.jsx Outdated Show resolved Hide resolved
changelog/_0003.md Show resolved Hide resolved
adhocracy4/reports/static/reports/ReportModal.jsx Outdated Show resolved Hide resolved
adhocracy4/reports/static/reports/ReportModal.jsx Outdated Show resolved Hide resolved
adhocracy4/comments_async/static/modals/UrlModal.jsx Outdated Show resolved Hide resolved
adhocracy4/comments_async/static/modals/UrlModal.jsx Outdated Show resolved Hide resolved
@hom3mad3
Copy link
Contributor Author

hom3mad3 commented Jan 23, 2025

Very good work, looks nice. It was interesting to see the dialog element being used. Haven't done that before. I left a some questions for you, but it looks good already.

dialog <3! it simplifies life a lot since its basically two methods open and close

my biggest issue so far, was that at some point i tested the finger swiper for mobile and that was basically adding half the existing lines of code just for that, so i decided to keep simple (also explains some leftover styles in the other branch hehe). it's a great little native component, but if you need fancy stuff then you gotta write a lot more.

i'm super happy not to have to write custom keyboard and accessibility extensions anymore though 😮‍💨

@hom3mad3
Copy link
Contributor Author

hom3mad3 commented Jan 23, 2025

@CarolingerSeilchenspringer @mcastro-lqd @vellip oh, i'll also see if i can already quickly change the current copy button to use the new "toggle" button. if not, i'll create an issue for after release.

@hom3mad3 hom3mad3 force-pushed the mr-2025-01-redesign-dialogs branch 2 times, most recently from cdae12c to fe4becb Compare January 27, 2025 10:45
@hom3mad3 hom3mad3 requested a review from vellip January 27, 2025 10:48
Copy link
Collaborator

@vellip vellip left a comment

Choose a reason for hiding this comment

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

LGTM, just the useId thing

@hom3mad3 hom3mad3 force-pushed the mr-2025-01-redesign-dialogs branch 2 times, most recently from 307cd1e to 1bf3785 Compare January 29, 2025 15:45
@hom3mad3 hom3mad3 force-pushed the mr-2025-01-redesign-dialogs branch from 1bf3785 to 13c19b7 Compare January 29, 2025 15:51
@hom3mad3 hom3mad3 merged commit 4e93a96 into main Jan 30, 2025
5 checks passed
@hom3mad3 hom3mad3 deleted the mr-2025-01-redesign-dialogs branch January 30, 2025 16:41
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.

4 participants