blm: convert to button #3306
blm: convert to button #3306
Conversation
|
Is there a strong reason for this design change? I personally like the banner + button, I felt it drew attention to the message and broke up the page nicely. |
Well, it's hard to speak objectively about design, but the unused space on either side of the button doesn't look right to me.
I don't see a problem w/ having a banner + button.
The message appears to be missing from the banner. Currrently it's a button that has a hashtag for the button text. Perhaps we can put this button inside of an alert component. It would turn out to look like a full-width flash alert with action button. How about it? We would just need the message text. Thoughts? |
|
Closing due to lack of follow-up. |
|
@nodejs/tsc as discussed in the meeting today, please evaluate the two options and comment which one you prefer: https://user-images.githubusercontent.com/17770407/87524387-1cbf2f00-c656-11ea-8e94-7f80de3ed847.png |
|
I don't have a strong opinion but my personal preference is the 2nd one: https://user-images.githubusercontent.com/17770407/87524580-53954500-c656-11ea-8991-2935a8fcfdef.png |
|
Personally I don't think either one looks better than the other, and both fit the current site design more than a full alert component would (although I'm saying that without seeing how it would look like, so maybe I'm wrong). I agree with Myles that the current button+banner design draws more attention though. |
|
I still like the current design better personally. I feel like removing the spill on either side of the button leaves a lot of empty white space. With that said I don't feel strongly about it, so if this is important to someone I am happy to see the change land |
|
Discussed in TSC meeting and asked members to comment in the issue. |
|
I slightly prefer the current version, mostly due to the interaction with the hover state. |
|
I personally prefer the design change while I do believe that the old one draws more attention. As such I'll abstain. |
|
Discussion in TSC meeting today is that there are no objections to this landing, we can go ahead and land. |
|
Great, glad everyone is on board for a button. I believe I was able to address everyone's concerns in the latest commit. No JavaScript for the ARIA states since it's really just a link (possibly unused styles too, but I'm not going to sweat it). |
The commit improves the visual appeal of the BLM CTA.
Notes:
https://user-images.githubusercontent.com/17770407/87524387-1cbf2f00-c656-11ea-8e94-7f80de3ed847.png
https://user-images.githubusercontent.com/17770407/87524580-53954500-c656-11ea-8991-2935a8fcfdef.png
Refs: #3246
Post-Feedback: https://user-images.githubusercontent.com/17770407/92166219-3f73f580-ee06-11ea-893c-0017d5d00ad0.png
The commit improves the visual appeal of the BLM CTA.
Notes:
https://user-images.githubusercontent.com/17770407/87524387-1cbf2f00-c656-11ea-8e94-7f80de3ed847.png
https://user-images.githubusercontent.com/17770407/87524580-53954500-c656-11ea-8991-2935a8fcfdef.png
Refs: #3246
Please consider either making the BLM CTA a full banner or simply a button as my PR has done.
/cc @MylesBorins