console icon indicating copy to clipboard operation
console copied to clipboard

fix: secret id overlap with secret key for large secret ids numbers

Open teyim opened this issue 6 months ago • 5 comments

:mag: Overview

When the number of secrets in a table is > 999, the Secret ID column overlaps with the Secret row column .Fixes #526

:bulb: Proposed Changes

Modify the styling of the Secret ID column to increase for IDs larger than 999

:framed_picture: Screenshots or Demo

Screenshot 2025-06-19 at 11 49 39 AM

:memo: Release Notes

fix secret id overlap with secret key for large secret ids numbers

:question: Open Questions

If there are aspects of the changes that you're unsure about or would like feedback on, list them here.

:test_tube: Testing

Describe the testing strategy. List new tests added, existing tests modified, and any testing gaps.

:dart: Reviewer Focus

Guide the reviewer on where to start the review process. Suggest specific files, modules, or functionalities to focus on as entry points.

:heavy_plus_sign: Additional Context

Provide any additional information that might be helpful for reviewers and future contributors, such as links to related issues, discussions, or resources.

:sparkles: 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.

:green_heart: Did You...

  • [-] Ensure linting passes (code style checks)?
  • [ ] Update dependencies and lockfiles (if required)
  • [ ] Update migrations (if required)
  • [ ] Regenerate graphql schema and types (if required)
  • [-] Verify the app builds locally?
  • [-] Manually test the changes on different browsers/devices?

teyim avatar Jun 19 '25 12:06 teyim

Hi @rohan-chaturvedi, thanks for the feedback, will do the modification accordingly

teyim avatar Jun 26 '25 19:06 teyim

@rohan-chaturvedi I did the modification as requested, and this is how it looks: Screenshot 2025-07-02 at 5 11 53 PM

Screenshot 2025-07-02 at 5 11 11 PM

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.

teyim avatar Jul 02 '25 16:07 teyim

@rohan-chaturvedi i did the modification as requested and this is how it looks: Screenshot 2025-07-02 at 5 11 53 PM Screenshot 2025-07-02 at 5 11 11 PM

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

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

rohan-chaturvedi avatar Jul 02 '25 16:07 rohan-chaturvedi

Hi @rohan-chaturvedi, your approach looks way cleaner. let me do the updates

teyim avatar Jul 02 '25 16:07 teyim

Screenshot 2025-07-03 at 12 51 25 PM Screenshot 2025-07-03 at 1 08 29 PM

@rohan-chaturvedi we Good

teyim avatar Jul 03 '25 12:07 teyim