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

Review and replace usage of float layouts in admin UI for RTL support #8126

Closed
1 of 12 tasks
thibaudcolas opened this issue Mar 15, 2022 · 4 comments · Fixed by #11360
Closed
1 of 12 tasks

Review and replace usage of float layouts in admin UI for RTL support #8126

thibaudcolas opened this issue Mar 15, 2022 · 4 comments · Fixed by #11360
Assignees
Labels
component:Design system Including the pattern library (Storybook) component:i18n i18n for content created in Wagtail, not the admin UI itself type:Bug

Comments

@thibaudcolas
Copy link
Member

thibaudcolas commented Mar 15, 2022

Follow-up to #8051, based on @wagtail/ui team discussions (LB - Thibaud in particular), and research from @fabienheureux. Wagtail’s admin UI overuses float/clear for layout. This is problematic for RTL support since there is no browser support for flow-relative float/clear values. We could technically refactor our usage of float to have overrides based on [dir=rtl] attribute selectors, but floats tend to be hard to work with anyway, so we expect it would be better for us to instead focus on refactoring float usage to CSS grid or flexbox.

2023-02-17 review of the styles: 22 float: left, 12 float: right, 8 occurrences of clearfix mixin, 13 clear.

  • Legacy dropdown
  • Footer actions (New styles for footer actions #9030)
  • content-wrapper
  • header
  • Listings filters
  • Listings pagination
  • Listings bulk actions
  • ModelAdmin placeholder content
  • jQuery datetimepicker
  • Comments actions
  • Tag filter (@column)
  • Column classes

Original assessment:

  • 21 clear: properties set
  • 21 places our clearfix mixin is used
  • 83 float: properties set

All of those instances need to be reviewed, and either removed or refactored to CSS grid / flexbox. We should then use stylelint to enforce no new code uses floats for layout.


For anyone wanting to contribute to this, there are two options:

  • Spend time reviewing the code, and comment to this issue with your findings / recommendations for a particular case we’re using float
  • Get on with refactoring for a given bit of the admin UI
@thibaudcolas thibaudcolas added type:Bug component:i18n i18n for content created in Wagtail, not the admin UI itself component:Design system Including the pattern library (Storybook) labels Mar 15, 2022
@thibaudcolas
Copy link
Member Author

Approach we discussed with @fabienheureux:

  1. Set up visual regression testing w/ https://github.com/thibaudcolas/wagtail-tooling
  2. Try to remove all floats and see how much of the potential regressions are picked up
  3. Then work through the different float usage one by one, updating to Flexbox or grid

Components to ignore:

  • Anything form styles / page editor / footer actions
  • Datetimepicker
  • Header
  • Chooser

Potentially to trial first: 404, ModelAdmin

@thibaudcolas
Copy link
Member Author

As of #8921, we have:

  • 8 instances of our clearfix mixin. Most of them are related to our legacy float grid, which could likely be switched over to CSS grid layout without further refactoring
  • 10 instances of clear
  • 44 instances of float

@lb-
Copy link
Member

lb- commented Feb 7, 2023

#10020 - removes float usages from the gravatar / user profile image switching.

@thibaudcolas
Copy link
Member Author

As of #8520, I believe our browser support will allow us to use flow-relative float: inline-start / float: inline-end. I’m still keen to eventually remove all usage of float but in the meantime it’ll be great for us to switch to logical values for this.

Audit of float styles currently:

  • 43 instances of float
  • 10 clear
  • 7 clearfix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:Design system Including the pattern library (Storybook) component:i18n i18n for content created in Wagtail, not the admin UI itself type:Bug
Projects
Status: Done
Status: In Progress
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants