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

Global link colour overriden when including sass #97

Open
timcowlishaw opened this issue Mar 4, 2024 · 3 comments
Open

Global link colour overriden when including sass #97

timcowlishaw opened this issue Mar 4, 2024 · 3 comments

Comments

@timcowlishaw
Copy link

Hi there folks!

I don't fully get what's going on here, but i had a weird issue when including tna-frontend in nationalarchives/ds-caselaw-public-ui in that the tna-frontend css globally overrode our link style with an undefined variable, leading to all links showing as black:

image

(note that the titles of judgments are black, as are the unselected pagination links, rendering them effectively invisible).

I haven't been able to track down exactly where in tna-frontend that color: var(--link) directive comes from - bizarrely it also seems to take precedence over other more specific rules in our own codebase - even if i set --link to our global link colour before including the tna-frontend css, it still seems to take precedence over other more specific selectors in our own css? I've managed to get around this by sprinkling some !importants around our codebase, but would be good to understand a bit better what's actually going on.

thank you!

Tim

@ahosgood
Copy link
Member

ahosgood commented Mar 4, 2024

The colour of the links is set by the selected theme.

You will need a tna-template class in the <html> element as well as either:

  • tna-template--system-theme or
  • tna-template--light-theme or
  • tna-template--dark-theme

This will then set all the CSS custom properties we need for colours:

image

Without these classes, I imagine the links will default to black because there is no variable set.

@timcowlishaw
Copy link
Author

Hi there Andrew,

Aha, thanks for this - I've added the tna-template--system-theme class and saw some improvement, and was able to take care of all the outstanding link colour weirdnesses by adding a few !importants to our rules where their specificity wasn't enough for them to take precedence, however this revealed a few other issues - there's quite a few unscoped rules which get included, in particular, (aside from the global link colours) the following caused us problems:

The global reset of margin and padding at https://github.com/nationalarchives/tna-frontend/blob/main/src/nationalarchives/utilities/_typography.scss#L71-L74

The setting of all images to be display: block; by default in https://github.com/nationalarchives/tna-frontend/blob/main/src/nationalarchives/utilities/_reset.scss

Would it be impractical to scope all the rules defined by tna-frontend below some "master class" eg ".tna-frontend" that could be applied to any element which uses elements of the design system (or indeed to the html element if being used globally, as with the theme class). It'd make gradual adoption of the design system by existing projects a lot simpler!

Thank you!

Tim

@ahosgood
Copy link
Member

ahosgood commented Mar 6, 2024

The resets for images, videos, canvases etc. are only element specificity (0-0-1) which means they can be overwritten with a single class (0-1-0). If we were to scope them inside another class (e.g. .tna-template img 0-1-1) then overwriting those styles for an image within a component using BEM (e.g. .tna-card__image 0-1-0) wouldn't be possible without more specific selectors.

The same applies to the * selector - scoping that makes it more important and then harder to override. There are also quite a number of elements that benefit from the margin: 0; padding: 0; reset and I don't think we could remove that without quite a bit of work and testing.

The main consumers of TNA Frontend will be creating new services using the library rather than migrating existing ones. Given that there are so many different possible styles and libraries that other services could be using I think we could potentially spend too long trying to make it more seamless for a number of one-time migrations when instead we could be improving and adding to the components we have. We also have to be aware that we are a very small and focused team.

I need to have a think about this and what the best way forward is.

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

No branches or pull requests

2 participants