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.

Sign up for GitHub

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

Jump to bottom

feat: Access team secondary colour via MUI theme #2646

Merged
merged 6 commits into from
Jan 12, 2024
Merged

feat: Access team secondary colour via MUI theme #2646

merged 6 commits into from
Jan 12, 2024

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Jan 9, 2024 •
edited
Loading

What does this PR do?

  • Update team_themes table to include action_colour and link_colour columns, and drops secondary_colour column
  • Pulls these new values into the MUI theme
  • Imports Team and associated types from planx-core
  • Depends upon chore: Update team_themes types planx-core#259

Not in this PR

  • No style changes are made
  • link_colour value in the database is set to the current default
  • color="prompt" is not used anywhere yet so this value is not pulled in and used
  • @ianjon3s is planning to pick up these changes (thank you!)

If you wish to do so, you can test this locally by updating colours for a team in Hasura and then see them get picked up in /preview and /unpublished routes. You'll need to set color="prompt" on a component (e.g. button) to see the action_colour feeding through.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 9, 2024 •
edited
Loading

Removed vultr server and associated DNS entries

@DafyddLlyr DafyddLlyr force-pushed the dp/team-secondary-colour-mui-theme branch from 851d8a6 to 22266c2 Compare January 10, 2024 17:00
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 10, 2024 •
edited
Loading

🤖 Hasura Change Summary compared a subset of table metadata including permissions:

Updated Tables (1)

  • public.team_themes permissions:

    insert select update delete
    api ➕/➖
    platformAdmin ➕/➖ ➕/➖
    public ➕/➖
    teamEditor ➕/➖ ➕/➖
    18 updated column permissions
    insert select update
    api ➕ action_colour
    ➕ link_colour
    ➖ secondary_colour
    platformAdmin ➕ action_colour
    ➕ link_colour
    ➖ secondary_colour
    public
    teamEditor ➕ action_colour
    ➕ link_colour
    ➖ secondary_colour

@DafyddLlyr DafyddLlyr force-pushed the dp/team-secondary-colour-mui-theme branch 3 times, most recently from 0911bf6 to f38844d Compare January 11, 2024 09:46
DafyddLlyr
DafyddLlyr commented Jan 11, 2024
View reviewed changes
editor.planx.uk/src/theme.ts

const DEFAULT_PRIMARY_COLOR = "#0010A4";
const DEFAULT_TONAL_OFFSET = 0.2;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No change here, this is just the MUI default we're currently using.

Docs: https://mui.com/material-ui/customization/palette/#tonal-offset

DafyddLlyr
DafyddLlyr commented Jan 11, 2024
View reviewed changes
editor.planx.uk/src/theme.ts
Comment on lines +47 to +52
prompt: {
main: DEFAULT_PRIMARY_COLOR,
contrastText: "#FFFFFF",
light: lighten(DEFAULT_PRIMARY_COLOR, DEFAULT_TONAL_OFFSET),
dark: darken(DEFAULT_PRIMARY_COLOR, DEFAULT_TONAL_OFFSET),
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an augmentColor() utility to handle this automatically, but it only happens after a theme has been generated so I'm doing this manually here.

Docs: https://mui.com/material-ui/customization/palette/#generate-tokens-using-augmentcolor-utility

DafyddLlyr
DafyddLlyr commented Jan 11, 2024
View reviewed changes
scripts/seed-database/write/team_themes.sql
@@ -1,3 +1,5 @@
-- TODO: Update to add action_color and link_colour columns
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Intentional TODO, will need to come back to pick this up post-deploy to production.

DafyddLlyr
DafyddLlyr commented Jan 11, 2024
View reviewed changes
editor.planx.uk/src/themeOverrides.d.ts
@@ -25,10 +25,14 @@ declare module "@mui/material/styles" {
declare module "@mui/material/styles/createPalette" {
interface Palette {
border: { main: string; input: string; light: string };
link: { main: string; };
prompt: { main: string; contrastText: string; light: string; dark: string; };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll be honest, not a huge fan of prompt but it was the best worst option here 😅 Very open to other suggestions.

"Action" is already used by MUI in a different context, "Button" feels a bit odd / meaningless -

<Button color="button">
  Continue
</Button>

@DafyddLlyr DafyddLlyr marked this pull request as ready for review January 11, 2024 10:40
@DafyddLlyr DafyddLlyr requested a review from a team January 11, 2024 10:40
@DafyddLlyr DafyddLlyr mentioned this pull request Jan 11, 2024
Merged
@DafyddLlyr DafyddLlyr force-pushed the dp/team-secondary-colour-mui-theme branch from f38844d to 23e8eae Compare January 11, 2024 11:57
@DafyddLlyr DafyddLlyr merged commit e840e7d into main Jan 12, 2024
12 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/team-secondary-colour-mui-theme branch January 12, 2024 10:53
@ianjon3s ianjon3s mentioned this pull request Jan 12, 2024
Closed
DafyddLlyr added a commit to theopensystemslab/planx-core that referenced this pull request Jan 12, 2024
@DafyddLlyr
chore: Update team_themes types (#259)
4589660 
## What does this PR do?
- Updates types to reflect changes implemented in
theopensystemslab/planx-new#2646
- Exports `Team` types
@ianjon3s ianjon3s mentioned this pull request Jan 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jessicamcinchak jessicamcinchak jessicamcinchak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DafyddLlyr @jessicamcinchak