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

Migrate away from _utilities.legacy.scss #8947

Closed
lb- opened this issue Aug 4, 2022 · 14 comments · Fixed by #11365
Closed

Migrate away from _utilities.legacy.scss #8947

lb- opened this issue Aug 4, 2022 · 14 comments · Fixed by #11365

Comments

@lb-
Copy link
Member

lb- commented Aug 4, 2022

Is your proposal related to a problem?

  • For contributors it is currently not clear what utilities classes should be used as some code uses the Tailwind classes and some uses the legacy utility classes.
  • It would be good to remove the client/scss/overrides/_utilities.legacy.scss file and replace those usages with the appropriate Tailwind classes OR remove their usage completely (e.g. .clear).
  • Note: There is confusion also about when to use Tailwind classes for a whole component vs just a sub-set of adjustments, but that can be covered elsewhere.

Describe the solution you'd like

Remove the utilities legacy file and replace each usage if needed with either SCSS mixins OR the equivalent tailwind utility class.

class Note
.u-hidden Replace with w-hidden - but validate each usage
.clearfix Remove, replace with mixin clearfix() if used anywhere else, will be removed completely in the near future
.nice-padding Convert to a Tailwind utility
.divider-before Remove, not used
.divider-after Review, may not be needed and does not add anything useful
body.reordering Should probably be in layouts
.show-transparency Convert to Tailwind utility
.inline Replace usage with w-inline (validate each usage)
.inline-block Replace usage with w-inline-block (validate each usage)
.block Replace usage with w-block (validate each usage)
.unlist Remove, only current usage is in Styleguide and this example could either removed or replaced with Tailwind classes
.overflow Replace usage with appropriate w-overflow-... Tailwind class (validate each usage) OR remove if not used

Describe alternatives you've considered

  • Leave as is, each contributor can just keep using these for now.

Additional context

@lb-
Copy link
Member Author

lb- commented Aug 4, 2022

@thibaudcolas any thoughts on this?

@lb-
Copy link
Member Author

lb- commented Aug 4, 2022

Created after reviewing #8855 - new code added using .u-hidden class.

@thibaudcolas
Copy link
Member

All of this makes sense to me. Note u-hidden uses !important, so the logical Tailwind replacement would be !w-hidden.

@Lovelyfin00
Copy link
Contributor

Hi @thibaudcolas
Is this issue still open?

@lb-
Copy link
Member Author

lb- commented Oct 22, 2022

Go for it @Lovelyfin00 - this is still open. Try to address a few small ones in one PR before you tackle them all.

A good place to start is just the removal of unused ones. Be sure you check the code yourself, and also search for partial matches in the code.

@Lovelyfin00
Copy link
Contributor

Lovelyfin00 commented Oct 26, 2022

Hi @lb-
After I remove the classes that needs to be removed from the utitlities.scss file, where do I have to go in the codebase to find out where those classes are being used?

@lb-
Copy link
Member Author

lb- commented Oct 26, 2022

@Lovelyfin00 best to search the code for any usage.

Tailwind classes get generated based on the tailwind config file https://github.com/wagtail/wagtail/blob/main/client/tailwind.config.js

@Lovelyfin00
Copy link
Contributor

I can't find where these html are displayed on the wagtail bakery. I have searched edit post on the baskery admin but I still can't find what I'm looking for.

/wagtail/wagtail/documents/templates/wagtaildocs/documents/edit.html

/wagtail/wagtail/snippets/templates/wagtailsnippets/snippets/edit.html

@lb-
Copy link
Member Author

lb- commented Oct 26, 2022

@Lovelyfin00
Copy link
Contributor

Thank you so much @lb-
Last one for tonight I promise 😅😅

/wagtail/wagtail/contrib/forms/templates/wagtailforms/list_submissions.html

@lb-
Copy link
Member Author

lb- commented Oct 26, 2022

@Lovelyfin00 in the menu click forms - then click the contact form responses listing.

@lb-
Copy link
Member Author

lb- commented Oct 27, 2022

File: client/src/tokens/typography.jsAdd (maybe after lineHeight)

const listStyleType = {
  none: 'none',
};

Then ensure you export it at the bottom

module.exports = {
  systemUIFontStack,
  monoFontStack,
  fontFamily,
  fontSize,
  fontWeight,
  letterSpacing,
  lineHeight,
  listStyleType,
  typeScale,
};

Then import that listStyleType in client/tailwind.config.js

const {
  fontFamily,
  fontSize,
  fontWeight,
  letterSpacing,
  lineHeight,
  listStyleType,
  typeScale,
} = require('./src/tokens/typography');

Then include it in the module export

    fontWeight,
    lineHeight,
    listStyleType,
    letterSpacing,
    borderRadius,

Based on https://tailwindcss.com/docs/list-style-type

@lb-
Copy link
Member Author

lb- commented Nov 7, 2022

@Lovelyfin00 just a heads up divider-before is not used but divider-after is used in multiple places and serves a purpose.

Screen Shot 2022-11-08 at 8 26 44 am

See the line between the 'filesize/usage' column.

We possibly could migrate that to Tailwind utility classes but just flagging based on feedback on #9535

lb- pushed a commit to Lovelyfin00/wagtail that referenced this issue Nov 7, 2022
- `divider-before` not used
- add w-list-none to Tailwind utility classes
- remove now unused unlist class
- relates to wagtail#8947
lb- pushed a commit that referenced this issue Nov 7, 2022
- `divider-before` not used
- add w-list-none to Tailwind utility classes
- remove now unused unlist class
- relates to #8947
thibaudcolas added a commit to thibaudcolas/wagtail that referenced this issue Jan 9, 2024
thibaudcolas added a commit that referenced this issue Jan 9, 2024
@thibaudcolas
Copy link
Member

As of #11365, all that’s listed here has been changed except show-transparency and nice-padding. I’m not entirely sure what to do with those two so will hold off creating a further issue. show-transparency could probably stay as-is, and for nice-padding I have a feeling it could become a "component".

@thibaudcolas thibaudcolas added this to the 6.0 milestone Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants