Reinstate some default styling (tooltip, fieldsets) #4873
Conversation
sergei-maertens
commented
Nov 29, 2024
sergei-maertens
commented
- Removed the custom tooltip styling to be aligned with default-project again
- Fixed the checkbox markup in our react components to be consistent with rest of django admin
- Removed CSS overrides that have been obsoleted now that RJSF has been dropped.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4873 +/- ##
=======================================
Coverage 96.62% 96.62%
=======================================
Files 757 757
Lines 25945 25945
Branches 3419 3419
=======================================
Hits 25069 25069
Misses 613 613
Partials 263 263 ☔ View full report in Codecov by Sentry. |
|
I really like the changes (the big red minus and the accessibility improvements 👌) There are some weird spacing styling issues with the help texts that, in my opinion, should be 'fixed'. Things like:
I've added some comments in the chromatic to highlight these items Edit: this would again add some custom styling code.. |
sergei-maertens
force-pushed
the
a11y/reinstate-default-tooltips
branch
3 times, most recently
from
b86a737 to
21bc4e2
Compare
There was a problem hiding this comment.
This was... very broken since a long time :) I can't even remember if we had storybook back when we were on Django 3.2
sergei-maertens
force-pushed
the
a11y/reinstate-default-tooltips
branch
3 times, most recently
from
00457c1 to
ec75925
Compare
|
@robinmolen this should now be ready for re-review. Note that I dismissed some valid remarks due to it being the way it is in Django markup/CSS and I'd rather not introduce custom CSS again with all the pain it brings when upgrading Django :/ |
This aligns the implementation again with default project. The help text is the only way to specify crucial information/context or requirements for a given form field, and thus should be visible. Tooltips serve a different purpose - they can provide additional, informational, context but may not hide essential information for accessibility reasons. Unfortunately, the Django admin doesn't have a mechanism for using both flavours of information.
Left-over from the Django 4.2 upgrade where the HTML was changed. This ensures the help text is below the checkbox, rather than next to it with some awkward horizontal space between.
Without wrapping, the delete icon cannot be reached to delete a step because the step name is pushing it behind the form step details.
This used to be required to deal with styling from rjsf, but that dependency has been removed so we can stick to the normal Django styling instead.
The fieldboxes/multiple fields on a row was broken since the Django 4.2 upgrade because the markup changed extensively. The markup now again matches a vanilla Django admin. Fielboxes are slightly different since they gain additional wrappers, and the layout of the errors relative to the input/help text changes.
sergei-maertens
force-pushed
the
a11y/reinstate-default-tooltips
branch
from
ec75925 to
3bc48aa
Compare
