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

fix(ui): profile name gets cut off by roles #2915

Merged
merged 2 commits into from
Aug 13, 2024

Conversation

Christopher-Hayes
Copy link
Contributor

Description

Fixes the UI issue with the profile name getting cut off when the user has roles.

On the bottom-left profile card, the roles UI was moved to be below the username.

It could be in-between the name and username, but because this is a list of roles, for some users that list would look better as the last list item.

Minor tweaks

For this taller profile info card, the avatar was centered. AccountHoverCard.vue originally was not using the hoverCard prop on AccountInfo.vue, this was added since the hover card has the space to show roles inline with the profile name.

Screenshots

Before:

image

After:

image

The hover card remains untouched since it has the extra space:

image

Mobile:

image

Multiple users:

image

Misc

  • Linted
  • Passed tests

fixes: #2914

Copy link

netlify bot commented Aug 8, 2024

Deploy Preview for elk-docs canceled.

Name Link
🔨 Latest commit 5764846
🔍 Latest deploy log https://app.netlify.com/sites/elk-docs/deploys/66b7c5c569c6b60008078abe

Copy link

netlify bot commented Aug 8, 2024

Deploy Preview for elk-zone ready!

Name Link
🔨 Latest commit 5764846
🔍 Latest deploy log https://app.netlify.com/sites/elk-zone/deploys/66b7c5c5c250bd000853a0d5
😎 Deploy Preview https://deploy-preview-2915--elk-zone.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Christopher-Hayes
Copy link
Contributor Author

Christopher-Hayes commented Aug 8, 2024

Update - I just realized the AccountInfo card only shows 1 role. So, my comment about this being a "list of roles" doesn't apply.

I am also noticing the Elk preview link does not show any roles at all, no matter where they show in the app. Not sure why that is.

@ayoayco
Copy link
Member

ayoayco commented Aug 10, 2024

If I remember correctly, you'd only see roles if the profile is viewed in the oroginal server, and maybe the preview deployment is in a different server than your account.

@ayoayco
Copy link
Member

ayoayco commented Aug 10, 2024

I think the idea was for the profile to be consistent in different views. I'm not against moving the role chip down to prevent the name getting cut but maybe we should do that everywhere if we do it.

@Christopher-Hayes
Copy link
Contributor Author

Sure. So, if Elk went with this UI update, profile styling across all locations in the app would be:

<Display Name> <Lock Indicator> <Bot Indicator>
<Account Handle>
<Account Roles>

I pushed a commit to remove the code that shows the profile roles inline on the hover card. I also updated the profile page to make name + username + role order consistent with everywhere else that the profile is shown.

Profile hover card:
image

Profile page:
image

If we use this order, it gives the profile page room to show a full list of roles without the username getting pushed down. This is also the order that Mastodon uses.

image

@Christopher-Hayes
Copy link
Contributor Author

Christopher-Hayes commented Aug 10, 2024

I'm not specifically against putting the roles inline, it does vibe with Elk wanting to make the UI feel cleaner and more space efficient.

For a UI comparison, GitHub has a pretty clean hover card for profiles, with display name and pronouns inline.

image

At the end of the day, I'm just interested in fixing the profile UI at the bottom-left.

So, another alternative could be to just not show the roles at the bottom-left. It's not a public-facing part of the UI (compared to the profile page), and it's meant to tell the user what account they're logged into, rather than educate the user about their account. It could be argued that omitting roles from this UI would not confuse users too much, and on the consistency side, this would allow Elk to keep the inline roles everywhere else.

Copy link
Member

@antfu antfu left a comment

Choose a reason for hiding this comment

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

I am good with the change. We could figure out a better design later (if ever need), consider this is more like a fix, I think we could have it and move on first.

@shuuji3 shuuji3 added this pull request to the merge queue Aug 13, 2024
Merged via the queue into elk-zone:main with commit 71369c4 Aug 13, 2024
13 checks passed
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.

[UI] Profile name cut off by roles on AccountInfo.vue
4 participants