[WIP] style: inconsistent button styles in antD confirmation modal#22367
[WIP] style: inconsistent button styles in antD confirmation modal#22367josephsawaya wants to merge 1 commit into
Conversation
Co-authored-by: Joseph Sawaya <josephsawaya938@gmail.com> Co-authored-by: Vedant Prajapati <vedant.prajapati@outlook.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️
We hope to see you in our Slack community too!
Codecov Report
@@ Coverage Diff @@
## master #22367 +/- ##
=======================================
Coverage 66.86% 66.87%
=======================================
Files 1846 1847 +1
Lines 70510 70560 +50
Branches 7723 7737 +14
=======================================
+ Hits 47144 47184 +40
- Misses 21364 21374 +10
Partials 2002 2002
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
I have to assume there are many other locations in Superset that are exhibiting this inconsistency if they Modal component itself isn't already styled this way. In general, patching up a singular instantiation of something is more difficult to maintain, since the approach will be copied/pasted in numerous locations. Have you tried taking the approach of modifying the Modal component (which is essentially a decorated version of the AntD modal, with a layer of Emotion styling) so that the correct styles can be added at the root, rather than overriding/bypassing things for a singular instance of the modal? CC @michael-s-molina @eric-briscoe @geido in case either of them have looked at the Modal component lately. |
rusackas
added
the
requires:design-review
|
Sure I can help with some guidance here anytime |
|
@josephsawaya let me know if my comments make sense, and if we can help further. If you happen to join the Slack workgroup, we can certainly chat there, too! |
|
cc @eric-briscoe regarding getting to the root of the inconsistency in the design system. |
|
@rusackas @josephsawaya I recently merged a PR that changes the style of Modal's pre-defined functions. I think we can close this PR given that |
|
Sorry, I got caught up in exams after making this PR.
Based on the above I can close the PR. |
4 participants
SUMMARY
This PR changes the confirmation modal button styling to match the button styling in the datasource modal. It achieves this by hiding the default buttons in the confirmation modal and creating two buttons with the correct styling that make the function calls on click as the default buttons. Fixes: #16775
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
From the original issue:
With the changes in this PR applied:
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION