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

Show explicit user name instead of avatar in lists. #4828

Merged
merged 1 commit into from
Apr 24, 2020
Merged

Conversation

arikfr
Copy link
Member

@arikfr arikfr commented Apr 24, 2020

What type of PR is this? (check all applicable)

  • Other

Description

In several places in the application we show only the user avatar instead of their name. In large organizations that don't use avatars this leads to long lists with random icons instead of names.

  • Updated all lists (queries, dashboards, alerts) to show explicit user name.
  • On the dashboard page showing the user name when hovering over the user avatar.

There is probably a better UX to be achieved by moving the user name to a second line below the object name, but compared to the effort this seemed like a quick win.

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Before After
Queries List After: Queries List
Dashboards List After: Dashboards List
Alerts List After: Alerts List
Dashboard Page

@restyled-io
Copy link
Contributor

restyled-io bot commented Apr 24, 2020

Hey there-

I'm a bot, here to let you know that some code in this PR might not
match the team's automated styling. I ran the team's auto-reformatting tools on
the files changed in this PR and found some differences. Those differences can
be seen in #4829.

Please see that Pull Request's description for more details.

Columns.dateTime.sortable({ title: "Last Executed At", field: "retrieved_at", orderByField: "executed_at" }),
Columns.custom.sortable((text, item) => <SchedulePhrase schedule={item.schedule} isNew={item.isNew()} />, {
title: "Update Schedule",
title: "Refresh Schedule",
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed for consistency with the page.

Columns.dateTime.sortable({ title: "Created At", field: "created_at" }),
Columns.duration.sortable({ title: "Runtime", field: "runtime" }),
Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't feel very useful so removed it to have more space for names.

@@ -41,7 +41,9 @@ function DashboardPageTitle({ dashboardOptions }) {
ignoreBlanks
/>
</h3>
<img src={dashboard.user.profile_image_url} className="profile-image" alt={dashboard.user.name} />
<Tooltip title={dashboard.user.name} placement="bottom">
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW: This event doesn't work on mobile since there's no "hover" event. Not a big deal but maybe something to address down the road.

Copy link
Member Author

Choose a reason for hiding this comment

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

On mobile you can click on it to see the tooltip.

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work on the latest iOS via iPhone XS. Does work on iPad.

Copy link
Member Author

Choose a reason for hiding this comment

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

It worked for me on iPhone X. 😮

Copy link
Contributor

Choose a reason for hiding this comment

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

I bet that even if this didn't work it wouldn't impact many users 🤷‍♂️

@arikfr arikfr merged commit 6ee9b43 into master Apr 24, 2020
@arikfr arikfr deleted the queries-list branch April 24, 2020 14:32
@gaecoli
Copy link
Member

gaecoli commented Aug 19, 2020

How to add sortable to 'Created By'? I add orderByfield 'created_by', it worded for query list but not for dabhoard list page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants