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

refactor: Remove makeStyles classes in favour of styled components #1707

Closed
wants to merge 39 commits into from

Conversation

DafyddLlyr
Copy link
Contributor

Boring and repetitive work which helped the long train journey pass well yesterday!

There may be a few concerns with conflicts against some of Ian's work but I'm happy to rebase this, or even cherry-pick out commits that we can take forward a bit at a time. I appreciate there's a decent amount here for one PR sorry.

Comment on lines +14 to +16
"@mui/material": "5.12.3",
"@mui/styles": "5.12.3",
"@mui/utils": "5.12.3",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This version bump fixed the annoying bug where the "Change" text within a button on the SummaryList wasn't correctly inheriting the font-family.

mui/material-ui#33621

However, upgrading right to the latest version (5.13.1) was causing issues which I spent a lot of time trying to resolve. This version just came out a few days ago so hopefully it's something that will get ironed out shortly.

@DafyddLlyr DafyddLlyr marked this pull request as ready for review May 22, 2023 14:43
@DafyddLlyr DafyddLlyr requested a review from a team May 22, 2023 14:43
Comment on lines +82 to +85
const classes = {
tabs: `${PREFIX}-tabs`,
tabIndicator: `${PREFIX}-tabIndicator`,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that this is retained because of lines 134-137. What function do classes still have on the Tab component after this change?

@DafyddLlyr DafyddLlyr marked this pull request as draft May 30, 2023 09:59
@DafyddLlyr
Copy link
Contributor Author

Converted to draft until #1648 is reviewed and merged, and I've then re-based this 👌

@DafyddLlyr
Copy link
Contributor Author

Update from dev call - I'll keep this open as a reference for a while, but cherry-pick out commits to split this up into a series of smaller and simpler PRs 🍒

@DafyddLlyr
Copy link
Contributor Author

Closed - over the next few days or weeks, I'll just cherry-pick into individual PRs as we go along 🍒

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.

None yet

2 participants