Skip to content
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

UI: Empty state styles #5758

Merged
merged 16 commits into from
Nov 16, 2018
Merged

UI: Empty state styles #5758

merged 16 commits into from
Nov 16, 2018

Conversation

joshuaogle
Copy link
Contributor

@joshuaogle joshuaogle commented Nov 10, 2018

Adds new styles for empty states to provide helpful information (where possible)

  • Hides filters when empty state is shown
  • Better copy for all error messages
  • A couple of places made it difficult to use the component because of conditionals, so I'm just adding in the proper html there
  • Fixes UI Gray color, possibly just an old version
  • Switches remaining "Create" actions to use "Add" for consistency

Entities

Old:

image

New:

image


Namespaces

Uses the generic <ListView> component, which makes it trickier to add actions but still has helpful text (unless anyone has an idea for that)

Old:

image

New:

image

Copy link

@johncowen johncowen left a comment

Choose a reason for hiding this comment

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

Hey Joshua!

I only had a quick scan of this, I'm mainly commenting here for a question (but also to show a bit of ❤️ for owls 😄 ). Just on the wording change from Create to Add, we use a lot of Create wording in Consul, just wondering what the decision around this was as we could look to change all this in Consul also if that's preferred?

Little note, I've not approved, just because I haven't had a proper look really, I might come back later and give it a better look tho if nobody's got here before me.

👍

text-decoration: none;
}

> * + * {

Choose a reason for hiding this comment

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

Yeeesss! Lobotomised owl! (with one ear!!) 🙌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol, I hadn't heard that one yet. That's fantastic. It is a handy selector though, especially in nav lists like this

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah there was a great list apart article on it a few years back - https://alistapart.com/article/axiomatic-css-and-lobotomized-owls

@joshuaogle joshuaogle requested a review from a team November 12, 2018 15:27
@joshuaogle
Copy link
Contributor Author

Thanks @johncowen! Regarding the Add/Create language, I'm actually not sure. Add is more friendly and probably what they will be looking for, but we use Create in the docs so I don't want that mismatch. Mostly I want them to be consistent. I'll ping the product designers too to see what they think.

a,
.link,
a:not(.button):not(.file-delete-button):not(.tag) {
color: $blue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have these styles in a var or mixin somewhere? I feel like we use them in a few places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No we don't, but that's a good idea

@@ -1,11 +0,0 @@
<div class="box is-bottomless has-background-white-bis">
Copy link
Contributor

Choose a reason for hiding this comment

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

Byyyyyyeee 👋

@joshuaogle
Copy link
Contributor Author

Talked to the Product Designers and it sounds like we'll stick with "Create" for now but make sure things are consistent and do some testing around this question.

)
(concat
"No folders matching &quot;" this.filter "&quot;"
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👌 I thought this would look much more confusing

Copy link
Contributor

Choose a reason for hiding this comment

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

Heh yeah, still not great with concat and quoting various things, but not too bad!

@@ -57,7 +57,7 @@
<button type="button" {{action "toggleDisabled" item}} class="link">
Enable
</button>
{{else}}
{{else if (eq identityType 'entity')}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! Totally missed that

@@ -12,7 +12,7 @@
{{/link-to}}
</p.levelRight>
</PageHeader>
<ListView @items={{model}} @itemNoun="namespace" as |list|>
<ListView @items={{model}} @itemNoun="namespace" @emptyActions="empty-action-namespaces" as |list|>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Beautiful ✨

@joshuaogle joshuaogle merged commit f30e3db into master Nov 16, 2018
@joshuaogle joshuaogle deleted the ui-empty-states branch November 16, 2018 15:58
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.

3 participants