-
Notifications
You must be signed in to change notification settings - Fork 64
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
Documentation for the messages pattern #932
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced we need 2 examples if we only have a single list of messages. We can just display both states in the same example.
Also, @fordie have you built the example you're testing as a table
? Have you been accessibility testing for that? If not I suggest we move to div
s before it ends up becoming too hard to change later on.
@@ -0,0 +1,24 @@ | |||
.messages-table { | |||
.messages-table__row { | |||
.message-table__data { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These selectors shouldn't be nested as it makes the specificity too high.
One of the benefits of BEM is that it allows the targeted selection of elements (which reduces specificity) while avoiding naming collisions.
@@ -0,0 +1,23 @@ | |||
<table id="messagesTable" class="messages-table"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be a table
as it's not tabular data. It also won't collapse well on smaller viewports.
@@ -0,0 +1,23 @@ | |||
<table id="messagesTable" class="messages-table"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise here. This shouldn't be a table.
…ntend into SDT-813_messages-pattern
…ntend into SDT-813_messages-pattern
@rpowis no, I've been waiting for @ian-leggett to produce this version which he's run past Chris |
…ark Ford was using. Decided it wasn't needed.
Made a couple of changes to meet style.
Remove 'Example' from front of subjects
Remove 'Example' from front of subjects
Moved unread and read examples under 'Message list'.
…read out on my MAC using voice over.
generated from commit cbaf161
Problem
See issue - hmrc/design-patterns#44
Solution
Messages now displayed in a data table with two states - read and unread
Example Screenshot
Read
Unread