Skip to content

fix: show long permission names with ellipses and full text on hover#1893

Merged
perkinsjr merged 9 commits intounkeyed:mainfrom
DeepaPrasanna:fix/permission-names
Jul 23, 2024
Merged

fix: show long permission names with ellipses and full text on hover#1893
perkinsjr merged 9 commits intounkeyed:mainfrom
DeepaPrasanna:fix/permission-names

Conversation

@DeepaPrasanna
Copy link
Contributor

@DeepaPrasanna DeepaPrasanna commented Jul 11, 2024 •
edited
Loading

What does this PR do?

Fixes #1885

Attached video:
https://www.loom.com/share/7f3a5848d86f4bbf9f2f4e8ef1a41294?sid=424b1b28-c69e-449d-b588-7c6cece49cd8

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Use long permission names for example api.account.v1.AccountService.Accounts.DeleteConnections
  • Text should be truncated with an ellipsis and should show full text on hover

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

@changeset-bot
Copy link

changeset-bot bot commented Jul 11, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 4187f99

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jul 11, 2024

@DeepaPrasanna is attempting to deploy a commit to the Unkey Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 11, 2024 •
edited
Loading

Thank you for following the naming conventions for pull request titles! 🙏

@vercel
Copy link

vercel bot commented Jul 11, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 0:16am
planetfall ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 0:16am
workflows ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 0:16am
www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 0:16am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
play ⬜️ Ignored (Inspect) Visit Preview Jul 23, 2024 0:16am

@vercel vercel bot temporarily deployed to Preview – www July 11, 2024 17:51 Inactive
@vercel vercel bot temporarily deployed to Preview – planetfall July 11, 2024 17:52 Inactive
@vercel vercel bot temporarily deployed to Preview – workflows July 11, 2024 17:53 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard July 11, 2024 17:54 Inactive
@vercel vercel bot temporarily deployed to Preview – play July 11, 2024 17:54 Inactive
perkinsjr
perkinsjr requested changes Jul 11, 2024
View reviewed changes
Copy link
Member

@perkinsjr perkinsjr left a comment

Choose a reason for hiding this comment

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

This fixes it for roles but not permissions.

Permissions are still overflowing and or broken with extra long names

@DeepaPrasanna
Copy link
Contributor Author

DeepaPrasanna commented Jul 13, 2024 •
edited
Loading

did u mean here?
Screenshot 2024-07-13 223117
Untitled

@perkinsjr
Copy link
Member

perkinsjr commented Jul 13, 2024

did u mean here?

Screenshot 2024-07-13 223117

Untitled

Yes, that's our permissions section. As well as if you click into a permission to see the title and the name will span the entirety.

@DeepaPrasanna
Copy link
Contributor Author

DeepaPrasanna commented Jul 16, 2024 •
edited
Loading

are the following changes okay?

Screenshot 2024-07-16 224851

  1. Same UI, different scenarios
    Screenshot 2024-07-16 232029
    Screenshot 2024-07-16 232040
    Screenshot 2024-07-16 232053

image

Screenshot 2024-07-18 164320

Screenshot 2024-07-18 202807

Screenshot 2024-07-18 202652
Screenshot 2024-07-18 205324

@perkinsjr
Copy link
Member

perkinsjr commented Jul 16, 2024

@guilhermerodz can you take a look at this and decide how you want the dashboard to proceed.

@vercel vercel bot temporarily deployed to Preview – www July 16, 2024 18:50 Inactive
@perkinsjr Pullflow.com
Copy link
Member

perkinsjr commented Jul 16, 2024

cc Guilherme Rodz tagged you in this issue can you take a look, see how you feel about the changes.

@vercel vercel bot temporarily deployed to Preview – workflows July 16, 2024 18:51 Inactive
@vercel vercel bot temporarily deployed to Preview – planetfall July 16, 2024 18:51 Inactive
@vercel vercel bot temporarily deployed to Preview – play July 16, 2024 18:52 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard July 16, 2024 18:53 Inactive
guilhermerodz
guilhermerodz previously approved these changes Jul 18, 2024
View reviewed changes
Copy link
Contributor

@guilhermerodz guilhermerodz left a comment

Choose a reason for hiding this comment

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

Truncating with ellipses (three dots) is a great workaround for this.

There are still some alignment issues, such as title + description you showed on image 5 but that's not related to the truncated long names at all. Great job!

@guilhermerodz guilhermerodz dismissed their stale review July 18, 2024 19:06

Just found out description is missing truncate

guilhermerodz
guilhermerodz suggested changes Jul 18, 2024
View reviewed changes
Copy link
Contributor

@guilhermerodz guilhermerodz left a comment

Choose a reason for hiding this comment

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

Just missing a truncate className on the description of it
CleanShot 2024-07-18 at 16 05 45@2x
Add truncate and there you go
CleanShot 2024-07-18 at 16 05 36@2x

@DeepaPrasanna
Copy link
Contributor Author

DeepaPrasanna commented Jul 19, 2024

I added here too
image

@guilhermerodz guilhermerodz enabled auto-merge July 22, 2024 12:26
@guilhermerodz guilhermerodz disabled auto-merge July 22, 2024 12:26
@DeepaPrasanna DeepaPrasanna requested a review from perkinsjr July 23, 2024 09:33
@perkinsjr perkinsjr enabled auto-merge July 23, 2024 12:10
@vercel vercel bot temporarily deployed to Preview – planetfall July 23, 2024 12:14 Inactive
@vercel vercel bot temporarily deployed to Preview – workflows July 23, 2024 12:14 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard July 23, 2024 12:15 Inactive
@vercel vercel bot temporarily deployed to Preview – www July 23, 2024 12:16 Inactive
@perkinsjr perkinsjr added this pull request to the merge queue Jul 23, 2024
Merged via the queue into unkeyed:main with commit bfa49f6 Jul 23, 2024
@DeepaPrasanna DeepaPrasanna deleted the fix/permission-names branch July 23, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@perkinsjr perkinsjr perkinsjr approved these changes

@chronark chronark Awaiting requested review from chronark chronark is a code owner

+1 more reviewer

@guilhermerodz guilhermerodz guilhermerodz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

long permission names are cut off

3 participants

@DeepaPrasanna @perkinsjr @guilhermerodz