-
Notifications
You must be signed in to change notification settings - Fork 50
fix: secret id overlap with secret key for large secret ids numbers #592
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
base: main
Are you sure you want to change the base?
Conversation
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.
Hey @teyim , thanks for the PR! A few thoughts:
- This approach handles the increase to 4 digits (>999) but this wouldn't work for 5 digits. This is currently not likely, but a more general approach would be better than the current ad-hoc one. Ideally I would like this to be autoscaled using css rather than using a programmatic approach based on the number of secrets.
- This fix also doesn't address the secrets editor for individual environments. I think you'll need to make fixes in
SecretRow.tsx
:
Hi @rohan-chaturvedi, thanks for the feedback, will do the modification accordingly |
* chore: add updated demo video * chore: update demo video * chore: remove redundant assets * feat: 2x demo * feat: update cli ascii art * feat: add Terraform SVG icon * feat: update Phase Console demo image and refine secret sync description * test: video tag on gh * fix: remove video tag, fall back to attachment link * feat: add Amazon EKS SVG icon * docs: enhance README with additional repository links and refine CLI and secret management descriptions * feat: cleanup * docs: update secret sync description in README to reflect new icon * Update README.md Co-authored-by: Rohan Chaturvedi <rohan.chaturvedi@protonmail.com> * Update README.md Co-authored-by: Rohan Chaturvedi <rohan.chaturvedi@protonmail.com> --------- Co-authored-by: rohan-chaturvedi <rohan.chaturvedi@protonmail.com>
* fix: remove ad-hoc styles on delete service account button * fix: remove ad-hoc styles from DeleteServiceAccountTokenDialog
@rohan-chaturvedi I did the modification as requested, and this is how it looks: ![]() the issue now is the Key table heading does not perfectly align with the content of that key column. To make the Key table heading align properly, I will have to do something like so: const maxIndexDigits= get the length of the object array used to render the keys and extract the number of digits const indexWidthMap = { 1: 'min-w-5', 2: 'min-w-8', 3: 'min-w-10', 4: 'min-w-12', 5: 'min-w-14', }; const indexColWidth = indexWidthMap[maxIndexDigits] || 'min-w-5 I can then assign this class to a div/ Th before that of the key table heading, forcing the key table heading to push to the right as the number of key digits increase.. but again, this approach might have some downsides. |
Nice progress! Instead of a fixed map, how about something like: style={{ minWidth: `calc(${maxIndexDigits}ch + 1rem)` }} Just a suggestion. If you think the map approach is better, go ahead. We don't really need to scale arbitrarily. Upto 5 digits is already plenty |
Hi @rohan-chaturvedi, your approach looks way cleaner. let me do the updates |
![]() ![]() @rohan-chaturvedi we Good |
🔍 Overview
When the number of secrets in a table is > 999, the Secret ID column overlaps with the Secret row column .Fixes #526
💡 Proposed Changes
Modify the styling of the Secret ID column to increase for IDs larger than 999
🖼️ Screenshots or Demo
📝 Release Notes
fix secret id overlap with secret key for large secret ids numbers
❓ Open Questions
If there are aspects of the changes that you're unsure about or would like feedback on, list them here.
🧪 Testing
Describe the testing strategy. List new tests added, existing tests modified, and any testing gaps.
🎯 Reviewer Focus
Guide the reviewer on where to start the review process. Suggest specific files, modules, or functionalities to focus on as entry points.
➕ Additional Context
Provide any additional information that might be helpful for reviewers and future contributors, such as links to related issues, discussions, or resources.
✨ How to Test the Changes Locally
Give clear instructions on how to test the changes locally, including setting up the environment, any necessary commands, or external dependencies.
💚 Did You...