redesign btn - #412
Conversation
🦋 Changeset detectedLatest commit: bc80838 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
|
what if we made the underline a tad darker, like this? 
|
|
fyi @juruwolfe, I pushed a slight tweak to the documentation on this PR. I got rid of the note on adding the language button conditionally if translated embeds don't exist, and added a code block for it instead. |
MinamiFunakoshiTR
left a comment
MinamiFunakoshiTR
left a comment
There was a problem hiding this comment.
I'd make the underline a little darker but otherwise everything looks good
Yeah, I just wanted there to be more of a shift on hover, but maybe that's silly. @allyjlevine is going to take a look on Monday, so let's wait until then to merge |
|
To Julia's point about hierarchy, I think we can actually keep this button sentence-case instead of uppercase. We could keep it on same level as the byline/published date by keeping it sentence-case and using For the colors, could we change to What is the proposed margin for around the button? I'm curious how it will look by default when slotted between the headline and story text Could we make the Proposed changes look like this
|
| #translate-button:hover { | ||
| background-color: #889d9b; | ||
| color: #ebf7f7; | ||
| color: #333333; |
There was a problem hiding this comment.
Suggesting we use our design system variables. This would be var(--theme-colour-text-primary)
| letter-spacing: 0.06rem; | ||
| font-size: var(--theme-font-size-xs); | ||
| background-color: #d3e1e1; | ||
| color: rgb(91 101 101); |
There was a problem hiding this comment.
To keep with design system variables and match the byline, we can use var(--theme-colour-text-secondary)
| @@ -76,22 +76,20 @@ | |||
| text-transform: uppercase; | |||
There was a problem hiding this comment.
| text-transform: uppercase; |
| @@ -76,22 +76,20 @@ | |||
| text-transform: uppercase; | |||
| letter-spacing: 0.06rem; | |||
There was a problem hiding this comment.
| letter-spacing: 0.06rem; |
| color 0.25s ease; | ||
| transition: color 0.25s ease; | ||
| text-decoration: underline; | ||
| text-decoration-color: #afafaf; |
There was a problem hiding this comment.
| text-decoration-color: #afafaf; | |
| text-decoration-color: var(--tr-light-grey); |
| background-color: #889d9b; | ||
| color: #ebf7f7; | ||
| color: #333333; | ||
| text-decoration: underline; |
There was a problem hiding this comment.
| text-decoration: underline; | |
| text-decoration: underline; | |
| text-decoration-style: dotted; | |
| text-decoration-thickness: 1px; |
|
the changes look good to me! if julia is OK with them, @allyjlevine can go ahead and apply them |
3 participants
What's in this pull request
Redesigned the translation button to look more like a link (which it is, in practice) and generally make it more subtle/bring it down in the hierarchy. Curious if @allyjlevine has thoughts, I just thought the one on cuba was a bit intense to be the default for all stories.
Before
After