-
Notifications
You must be signed in to change notification settings - Fork 843
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
Add Writing guidelines #212
Conversation
Provides descriptions and examples of how to write UI text
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 love this! I found a few typos and had a couple of other suggestions.
<EuiPanel paddingSize="l"> | ||
<EuiText> | ||
<h3>Clear and concise</h3> | ||
<p>Get straight to the point—in a that way your users understand. Make every word contribute to meaning.</p> |
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.
"in a that way" -- typo?
|
||
<EuiSpacer /> | ||
<EuiSpacer /> | ||
<EuiSpacer /> |
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.
Instead of using three spacers, you can use a single extra-large one:
<EuiSpacer size="xxl" />
|
||
<EuiFlexItem> | ||
<EuiToast color="success" > | ||
<p>Didn't find what you were looking for?</p> |
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.
What do you think of adding a "don't" to contrast this with? I think it will make it easier for people to see how the lack of a contraction sounds awkward: "Did not find what you were looking for?"
|
||
<EuiFlexItem> | ||
<EuiToast color="success" > | ||
<p>You don't have any dashboards. Let's create some!</p> |
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 think a contrasting "don't" example here could also make it clear how too many exclamation points sounds weird. What do you think?
<EuiText> | ||
<p> | ||
<strong>Summarize the message in the title.</strong> | ||
Don’t use the words error, warning, confirm, or jaron such as oops and uh-oh. |
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.
Typo: "jaron" -> "jargon"
color="success" | ||
> | ||
For maximum compatibility, share the short URL of the snapshot. | ||
Not all wiki and markup parsers can the handle full-length version. |
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.
"parsers can the handle" -> "parsers can handle the"
<EuiToast | ||
color="success" | ||
> | ||
checkbox with label Show remaining slices |
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.
We can make this a checkbox like this:
<EuiCheckbox
onChange={() => {}}
id="checkbox1"
label="Show remaining slices"
/>
<EuiToast | ||
color="danger" | ||
> | ||
checkbox with label Show other |
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.
And here:
<EuiCheckbox
onChange={() => {}}
id="checkbox2"
label="Show other"
/>
Also fixed typos and added don't for contractions
I fixed the typos and added the code for the checkbox. Thanks, @cjcenizal The reason the Dos and Don'ts are at the bottom is so that the reader focuses on the text first, instead of the Dos and Don'ts. @cchaos and @snide looking forward to how you improve the design. Let me know if you have any comments about the content. |
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.
Just a few comments, but this is really great! So helpful to have one place to come get guidelines and see examples.
Just saw @snide's comment about design stuff so you can disregard any comments regarding that. I'm keeping in here just as a reminder.
<EuiFlexItem> | ||
<EuiToast color="success" > | ||
Number | ||
<EuiFieldText /> |
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.
We should use the correct implementation of form elements and their labels and help text like:
<EuiFormRow
id={makeId()}
label="Number"
helpText="Number must be between 1 and 5"
>
<EuiFieldText name="num" />
</EuiFormRow>
@gchaps Let me know if you want help doing that.
<EuiFlexItem> | ||
<EuiToast | ||
title="Uh-oh!" | ||
color="success" |
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.
This toast color should be danger.
<EuiFlexGroup> | ||
<EuiFlexItem> | ||
<EuiText> | ||
<p><strong>Avoid using "please."</strong> |
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.
Yes! 🥇
<EuiFlexItem> | ||
<EuiText> | ||
<p> | ||
<strong>Avoid the urge to explain |
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.
For the example of this one, can we show some self-explanatory fields that don't require help text? Then add another row using the example from this current one that talks about being too verbose in help text?
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.
@gchaps ^^ Mainly just this comment needs your attention
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.
@cchaos Sure I can add this information . Nothing bugs me more than seeing a form with fields for Name and Location and then showing placeholder text that says the same thing.
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.
Some minor comments and questions. This looks great. Don't worry too much about your layouts (or even your form examples). @cchaos and I can help out there.
<EuiFlexGroup> | ||
<EuiFlexItem> | ||
<EuiText> | ||
<p><strong>Use active rather than passive voice.</strong> It's more clear and less wordy.</p> |
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.
Should we link out to something about passive voice? Likely people are pretty far removed from high-school english class and could use a refresher on the difference between active and passive voice.
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.
@snide I'll give some quick primers on contractions, sentence case, and active vs. passive voice--or link to them. Also thanks for cleaning up the components--I know that the buttons in the mock dialog boxes are on the wrong side.
<EuiSpacer /> | ||
|
||
<EuiText> | ||
<h2>Capitalization</h2> |
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'd add an example in here that says when capitalization is OK (like proper nouns and product names). Usually people forget something like "Machine Learning" is a product of and a proper noun.
Also, I could be missing our usage here completely! Which is why it might be good to clarify it in more detail.
<EuiToast color="success" > | ||
Number | ||
<EuiFieldText /> | ||
Number must be between 1 and 5 |
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.
Note to myself for later. This should use the <FieldRow>
component.
Question for @gchaps though. Specifically for help text, what if you need more than two sentences? Should we say multi-sentence help text isn't allowed?
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.
@snide The UI should layer information. Do we have a component like signpost help? I can also add something about linking to documentation
<EuiFlexGroup> | ||
<EuiFlexItem> | ||
<EuiText> | ||
<p><strong>Use contractions</strong> if they make your text easier to comprehend.</p> |
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.
Again, I'd link to some primer on what contractions are. People might not know.
> | ||
Password | ||
<EuiFieldPassword defaultValue="password" /> | ||
Must be least 8 characters and include upper and lower case letters, numbers, and symbols such as !@#$%&. |
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.
Again, remind for myself to convert all this stuff to FormRow
.
I notice these help-text use punctuation. Intentional? Seems to contradict your earlier rule.
FWIW I think no punctuation is good for titles and statements, but I like them for form-label help text. Specifically because they often end up multi-sentence.
Added more detail and explanations
Ready for another review. |
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.
Fantastic! I have one last suggestion.
</p> | ||
</EuiText> | ||
</EuiFlexItem> | ||
<EuiFlexItem> |
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.
Can we find a different example for this point? Form labels are important usability and accessibility aids. They communicate the role of an input even after the placeholder text has gone (e.g. when the user has started typing), and screen readers use them to tell users what the input is for. I advocate for preserving labels in general. Maybe we can use an example which has unnecessary help text below the input instead?
One quick overall question, are we ok with having Do examples without a corresponding Don't and vice versa? |
size="s" | ||
onClick={() => window.alert('Button clicked')} | ||
> | ||
Remove index |
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 would think this should just be "Remove" not "Remove index"?
<EuiFlexGroup> | ||
<EuiFlexItem> | ||
<EuiText> | ||
<h3>Don't be clever wtih a serious message</h3> |
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.
Spelling - wtih -> with
Personally I really appreciate the contrast of a Don't with every Do. It helps me zero in on the point more quickly. |
Got it. I've added a few more don'ts |
Added some more don'ts and fixed examples
Provides descriptions and examples of how to write UI text