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

fix(button): defensively set hover/focus/active styles from legacy globals #205

Merged
merged 2 commits into from
Nov 27, 2019

Conversation

SiTaggart
Copy link
Contributor

We need to set some properties on buttons defensively when faced with legacy global styles in the likes of Console, when setting a button to be as an anchor.

a { ... }
a:hover { ... }

These override our styles as we rely on them not being set. Color and Text decoration seem to be the main ones I've spotted so far in console. There maybe more but we can add to the list one we find them.

Setting these explicitly should fix this for color and text-decoration

@vercel
Copy link

vercel bot commented Nov 27, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/twilio-dsys/paste/7vc5z25v1
🌍 Preview: https://paste-git-fix-buttonas-link-global-defence.twilio-dsys.now.sh

@aayushpi
Copy link
Contributor

How scalable is this approach?

@SiTaggart
Copy link
Contributor Author

How scalable is this approach?

For now this is fine. The approach I'm taking is that I'm only resetting the things we absolutely can't mitigate by the consumer rewriting their components. I don't think we should optimize for a world where you drop in a component into some bootstrap/legacy code soup and expect it to work, that definitely won't scale.

So for example, to get the buttons looking ok in console nav, I just removed a little unnecessary bootstrap that was surrounding it. That fixed most of it. We can't defend against legacy component / library styles, so the expectation should be set that you need to remove stuff that is breaking our component. But once that stuffs gone, I'm left with the global element styles that no one can get around. So we should defend against those.

There will be an upper limit where we won't be able to do much against legacy styling and make it a feasible product to maintain, so the burden does lay on the app owner to tidy up to allow seemless consumption of a new language.

@aayushpi
Copy link
Contributor

Are there scenarios where we’d have to remove these styles? Would these encourage people to continue applying global properties?

@SiTaggart
Copy link
Contributor Author

SiTaggart commented Nov 27, 2019

Are there scenarios where we’d have to remove these styles?

Nah, not outside of use changing the design of the component

Would these encourage people to continue applying global properties?

I doubt it, and if it does you've placed yourself firmly into "bad actor" category anyway

@SiTaggart SiTaggart merged commit 2a9ce57 into master Nov 27, 2019
@SiTaggart SiTaggart deleted the fix/button/as-link-global-defence branch November 27, 2019 20:21
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