Skip to content

update es-note #306

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

Merged
merged 3 commits into from
Apr 23, 2020
Merged

update es-note #306

merged 3 commits into from
Apr 23, 2020

Conversation

mansona
Copy link
Member

@mansona mansona commented Apr 16, 2020

This is essentially a "first pass" on the es-note design

Screenshot 2020-04-22 at 22 37 19

I created an issue to track the need for an update to this component: #307

@mansona mansona changed the base branch from upgrade to website-redesign-rfc April 16, 2020 13:56
@mansona mansona added the blocked 🛑 this issue is blocked by something else. label Apr 16, 2020
@mansona mansona force-pushed the es-note branch 3 times, most recently from d5f7ba4 to 0b73c14 Compare April 21, 2020 10:54
@mansona mansona mentioned this pull request Apr 21, 2020
@mansona
Copy link
Member Author

mansona commented Apr 22, 2020

@MelSumner you mentioned on #305 that the current design should be fine but that doesn't make any sense 🤔 there is no such thing as "the current design" any more for anything in the styleguide, as the first thing we did in this redesign process was clear out all of the CSS.

I have updated this PR so that it no longer depends on the new colour pallet and we can improve the design of the <EsNote /> component as part of #307 when we need it for other projects 👍

@mansona mansona marked this pull request as ready for review April 22, 2020 11:14
@mansona mansona removed the blocked 🛑 this issue is blocked by something else. label Apr 22, 2020
@mansona mansona requested review from a team April 22, 2020 11:16
@MelSumner
Copy link
Member

@mansona I'll be more precise next time- I meant the one that exists now on the guides and api docs. It might be helpful to be a little more generous in interpretation, though.

Concerns on this latest update:

  • does it stand out enough on the existing gray background?
  • do the two backgrounds have enough color contrast between them to pass color contrast checks?

If not, let's just add a brand border on the element. That will help it have enough color contrast to pass muster for now.

@mansona
Copy link
Member Author

mansona commented Apr 22, 2020

@MelSumner so as I mentioned before, this component will need a full design review before it can be used in any project (that's why I created #307) so this PR isn't really trying to get the EsNote ready to be used anywhere.

I updated the design to include the border-radius and border colour that the card uses to try and address the contrast issue (adding a brand colour was a bit too garish 🙈). If this isn't enough I'm happy to just switch it to the brand colour for now as it will be updated soon anyway 👍

@MelSumner
Copy link
Member

@mansona since a11y is a hard requirement for any code to merge, let's at least make sure even the thing that is merged today is conformant. We can def iterate from there.

@mansona
Copy link
Member Author

mansona commented Apr 22, 2020

@MelSumner I have bumped the border to the point that it passed the https://webaim.org/resources/contrastchecker/ 👍 I am now satisfied that this is conformant

Copy link
Member

@MelSumner MelSumner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 👍

@mansona mansona merged commit e8c460f into website-redesign-rfc Apr 23, 2020
@mansona mansona deleted the es-note branch April 23, 2020 07:34
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.

2 participants