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

ui: Fix dark borders on certain visualizations #11959

Merged
merged 2 commits into from
Jan 7, 2022

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Jan 6, 2022

We recently noticed that our CSS compression seems to remove the odd border color when we compile up the UI for embedding in the binary. This results in some borders being a lot darker than they should be, but only once embedded in the binary/production (so not within our Ember development environment).

Before:

Screenshot 2022-01-07 at 11 10 21

I noticed that just separating the border-color off from the shorthand border (which we do a lot anyway) seems to solve the issue without getting into too many ember-cli weeds. So this PR does that in the places I've noticed that are affected (topology and discovery chain visualizations), therefore fixing the issue and giving us time to figure out why this is happening if we want to do that (there's something to be said for just having a lint rule or something to make sure we always split the border-color, off which would make this problem go away).

After:

Screenshot 2022-01-07 at 11 03 26

Notes:

  • Our unPRed playwright based testing would catch/prevent this sort of thing.
  • I know there was an Ember CSS compression dependency which was very out of date, but the later version was not officially supported in Ember the last time I looked. That may have changed now. If that has changed, an upgrade could help.
  • Alternatively we could also go with an unsupported upgrade of the dependency and if that solves the issue, then go with that and ignore the unsupported-ness of it. I think my preferred option would be to stick to supported and if it's still an issue stick to the "always split off the border-color from the border" pattern. I like it being separate anyway (i.e. you can't do font: 12px bold red;)

@johncowen johncowen added the theme/ui Anything related to the UI label Jan 6, 2022
@johncowen johncowen marked this pull request as ready for review January 7, 2022 11:21
@johncowen johncowen requested review from amyrlam, natmegs, a user and jgwhite January 7, 2022 11:21
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging January 7, 2022 13:05 Inactive
@vercel vercel bot temporarily deployed to Preview – consul January 7, 2022 13:05 Inactive
@johncowen johncowen merged commit 1f8960d into main Jan 7, 2022
@johncowen johncowen deleted the ui/bugfix/border-color-compression branch January 7, 2022 16:15
@hc-github-team-consul-core
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/543075.

@hc-github-team-consul-core
Copy link
Contributor

🍒✅ Cherry pick of commit 1f8960d onto release/1.11.x succeeded!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants