-
Notifications
You must be signed in to change notification settings - Fork 84
Handle really long values in request manager #6917
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
Handle really long values in request manager #6917
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
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.
Greptile Overview
Greptile Summary
Added ellipsis with tooltip to LabeledText component to handle overflow of long identity and custom field values in the request manager.
- Added
ellipsis={{ tooltip: true }}and!max-w-60class toTextcomponent inLabeledText.tsxto truncate long values with hover tooltip - Changed gap props to use semantic values (
gap="small") for consistency - Updated
ListItem.tsxto use Tailwindgap-x-3 gap-y-2classes for different horizontal and vertical spacing
Confidence Score: 4/5
- This PR is safe to merge with minimal risk
- The changes are straightforward UI improvements for handling text overflow. One minor style guide deviation was noted where Tailwind CSS classes were used instead of Ant Design's native gap tuple syntax.
- No files require special attention - the changes are simple styling improvements
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| clients/admin-ui/src/features/privacy-requests/dashboard/list-item/components/LabeledText.tsx | 5/5 | Added ellipsis with tooltip and max-width constraint to handle long text values. Changed gap from numeric to semantic value. |
| clients/admin-ui/src/features/privacy-requests/dashboard/list-item/ListItem.tsx | 4/5 | Replaced Ant Design Flex gap prop with Tailwind CSS classes for different horizontal and vertical gaps, which may violate the style guide preference for using Ant Design components. |
2 files reviewed, 1 comment
|
|
||
| {hasExtraDetails && ( | ||
| <Flex wrap gap="middle"> | ||
| <Flex wrap className="gap-x-3 gap-y-2"> |
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.
style: Ant Design's Flex gap prop supports tuple values like gap={[12, 8]} for different horizontal and vertical spacing. Consider using gap={[12, 8]} instead of Tailwind classes to align with the style guide preference for Ant Design components.
Context Used: Rule from dashboard - Use Ant Design's Flex component before resorting to Tailwind CSS flexbox classes for layout. (source)
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.
It would be nice if it supported separate values for horizontal/vertical gaps, but it doesn't. From Ant's official docs:
gap: small | middle | large | string | number |
speaker-ender
left a comment
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.
Tested locally and looks good!
…est-manager-doesn-t-handle-long-values-properly
Ticket ENG-1862
Description Of Changes
Add ellipsis to identity/custom fields displaying in the new request manager page to handle really long values.
Code Changes
Steps to Confirm
b35f9f9a7fa84c81870480f62443b345dfe19b970b0f4114b3bd5f4b88a1b94f50a7639f48a1426fbc6c87f5b76a3db8You can also try a really long email name too.
Screenshot
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works