Rework styling using styled-components #2517
Conversation
paulmelnikow
added
frontend
|
|
I'll probably need to give this a second read, but here's some feedback after playing with it a bit on staging. This text should be in the centre instead of off to the left:
I find this text:
harder to read than this:
I think keeping the text left-aligned within the table cells would be better Truncating this text is unhelpful - it means we can't copy & paste it properly:
Should this:
be styled the same as this:
..and this:
|
|
Thanks for digging in.
The truncation happens in CSS so copy and paste should work… |
| @@ -42,98 +110,109 @@ export default class Usage extends React.PureComponent { | |||
| renderStyleExamples() { | |||
| const { baseUrl } = this.props | |||
| return ( | |||
| <table className="badge-img"> | |||
| <QueryParamTable> | |||
There was a problem hiding this comment.
Do you think this one would also look better with textAlign = 'left' ?
Yep. With you.. I still think this one is a bit of an odd design decision. Now that we're moving away from showing the full URL in the edit box (and in a future PR we'll move away from having an edit box at all), it would be useful to show the full URL of the badge we've generated somewhere in un-tuncated form. i.e: it would be nice to be able to see all of the thing I'm generating without having to copy and paste it into another window. Also, This content is really wide, but we're not truncating it:
This content is really narrow, but we are truncating it: If we want to keep the width constrained, perhaps it would be better to wrap it, or allow it to be expanded somehow? |
|
To evaluate this PR it's important to inspect what's being generated. We've talked about the difference between what's useful as a Shields user and as a Shields dev, since our use cases for this dialog are different. I'm not sure I see why a Shields user needs to inspect this as it's being built, vs clicking Copy URL or Copy Markdown when they're done. The truncation isn't new; it's the narrower width and the dots that are. They make it look deliberate instead of an error:
Since this is blocking other work and is going to change again, I don't want to get hung up on it. For now how about I make the box wider? |
|
I set the markup back to its original width, and turned off |
|
There is one more strange bit of alignment I've noticed:
|
|
Huh, strange. None of them are supposed to be over there. Probably the width change caused that. |
|
Huh, I can't reproduce that.
|
|
How strange. I've just taken these 2 screenshots on the staging deploy at https://shields-staging-pr-2517.herokuapp.com Firefox: Chromium Stable: Is the difference that |
|
I think it only changes the font size. I've done some monkeying around with the Does disabling the cache help? If you run locally can you isolate what's different? |
|
Ahhh actually might it be a window-size issue? |
The CSS in the project is relatively difficult to change. While it is very DRY, it relies heavily on inheritance. It's difficult to make changes in the markup modal without it also affecting styles elsewhere.
styled-components is one of the leading CSS-in-JS libraries. By reducing dependency on global state and CSS inheritance, styles become explicit and are easier to inspect and change. It's also convenient that styles can be embedded with the components they modify.
At runtime, the library creates CSS classes, so it's pretty efficient.
We were using a little bit of styled-jsx before, which ships with Next.js, though styled-components is more widely used and I've had good experiences with it all around.
In a few cases I've duplicated styles where it feels more natural to do that: for example,
text-align: centeris duplicated inMainandMarkupModal.Much of this is a refactor, though there are a few visual changes, particularly in the markup modal and the style examples:
Blocking further work on #2496.