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

Add detailed diffs of changes made to secrets #274

Open
rohan-chaturvedi opened this issue Jun 16, 2024 · 16 comments
Open

Add detailed diffs of changes made to secrets #274

rohan-chaturvedi opened this issue Jun 16, 2024 · 16 comments
Assignees
Labels
enhancement New feature or request fosshack Issues for FOSS Hack 2024 frontend Change in frontend code

Comments

@rohan-chaturvedi
Copy link
Member

Is your feature request related to a problem?

Making changes to secrets currently shows a "You have undeployed changes to this environment." warning banner and an amber highlight to affected secrets, but it can be difficult to determine what changes were made other than which secrets are affected.

2024-06-16.20-44-40.webm

Describe the solution you'd like

Clicking the warning banner should show a modal with details of secrets affected, with diffs of specific properties changed, such as tags, comments, etc. For example:

  • Modified GOOGLE_CLIENT_ID
    • changed key
    • added tag
  • AddedTEST
  • Modified SESSION_COOKIE_DOMAIN
    • changed comment

Ideally, something inspired by VSCode's diffs would be great:

image

@rohan-chaturvedi rohan-chaturvedi added enhancement New feature or request frontend Change in frontend code fosshack Issues for FOSS Hack 2024 labels Jun 16, 2024
@harshith-1008
Copy link
Contributor

hey, I would like to work on this issue. @rohan-chaturvedi

@rohan-chaturvedi
Copy link
Member Author

hey, I would like to work on this issue. @rohan-chaturvedi

Great, its all yours!

@harshith-1008
Copy link
Contributor

Screenshot 2024-07-11 at 12 09 50 PM Screenshot 2024-07-11 at 12 14 03 PM

Hey @rohan-chaturvedi just wanted a little clarification about the UI, for testing I have created a dropdown ui, should I make a separate pop up screen/page for the changes or should I just try to make the current dropdown look cleaner?

@rohan-chaturvedi
Copy link
Member Author

Hey @harshith-1008 , this is a good start! However I think we should avoid this "accordion" style behavior, since it shifts the entire rest of the UI down. Instead, how about trying a modal dialog? We have a GenericDialog component you could use: https://github.com/phasehq/console/blob/main/frontend/components/common/GenericDialog.tsx

@harshith-1008
Copy link
Contributor

thanks for the feedback, ill try to implement it.

@harshith-1008
Copy link
Contributor

@rohan-chaturvedi I tried using genericDialogue component, but it expects button variant props, if I pass button variant then the alert changes to green box around the text. I have changed the component such that it accepts an empty string as props for genericDialogue, is it fine?
Screenshot 2024-07-25 at 1 10 17 PM

@rohan-chaturvedi
Copy link
Member Author

@rohan-chaturvedi I tried using genericDialogue component, but it expects button variant props, if I pass button variant then the alert changes to green box around the text. I have changed the component such that it accepts an empty string as props for genericDialogue, is it fine? Screenshot 2024-07-25 at 1 10 17 PM

@harshith-1008 that should be fine, just make sure that its not a breaking change for other components that use it. I'll take a look in detail once you have a PR. Once thing you could try is adding an i icon in an outline or secondary button as the modal trigger rather than the entire alert text 🤔

@harshith-1008
Copy link
Contributor

Screenshot 2024-07-27 at 10 47 12 PM Screenshot 2024-07-27 at 10 47 23 PM

@rohan-chaturvedi I hope this UI is fine.

@rohan-chaturvedi
Copy link
Member Author

This is a great start! A few suggestions:

  • Add a bit more vertical spacing between items
  • Newly added secrets should show as "Added CHECKING" rather than "Modified 'Added CHECKING'"
  • Use MONOSPACE fonts for key names
  • Highlight the specific field that was changed, perhaps by using a higher font weight like font-medium or a different font color, (maybe font-mono as well?)
  • Explore the FontAwesome Icons and try and use some icons to denote modifications vs additions

@harshith-1008
Copy link
Contributor

Screenshot 2024-07-28 at 12 34 14 AM

@rohan-chaturvedi is this ui okay?

@rohan-chaturvedi
Copy link
Member Author

That looks awesome! A few notes:

  • Only use font-mono for key names or field names. For example: "Modified AWS_SECRET_ACCESS_KEY"

  • Make sure the color-coding logic is consistent. Green makes sense for a new secret, but for modified secrets there's both blue and yellow. I'd suggest just picking one (yellow makes more sense IMO)

  • Could we show details of the changes here? For example:

  • Modified AWS_SECRET_ACCESS_KEY

    • Changed value
      • aCRAMarEbFC3Q5c24pi7AVMIt6TaCfHeFZ4KCf/a -> JBgoiFGNdbutADANDPmjp[OMKOJHduafdKAD

You could use the secret history UI as a baseline for this as well:

image

@rohan-chaturvedi
Copy link
Member Author

@harshith-1008 The SecretPropertyDiffs component could be useful for rendering details of changes

@harshith-1008
Copy link
Contributor

@rohan-chaturvedi just one small clarification, the font-mono should be just on keys name right? not the properties name right?

@rohan-chaturvedi
Copy link
Member Author

Yep, I think that will work well

@harshith-1008
Copy link
Contributor

Screenshot 2024-07-31 at 11 00 50 AM

@rohan-chaturvedi Is this UI fine?

@rohan-chaturvedi
Copy link
Member Author

@harshith-1008 This is good progress! The UI still needs a bit of work though. Could you tag me on the PR when its ready? It will be easier to suggest changes to the styles.

rohan-chaturvedi added a commit that referenced this issue Aug 31, 2024
* Secret changes dialog added

* added license

* Update and rename LICENSE.txt to LICENSE

* figma design changes

* fixed font irregularities

* fixed font

* renamed getclientSecrets to its original name

* Created DeployPreview component

---------

Co-authored-by: Nimish <85357445+nimish-ks@users.noreply.github.com>
Co-authored-by: Rohan Chaturvedi <rohan.chaturvedi@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fosshack Issues for FOSS Hack 2024 frontend Change in frontend code
Projects
None yet
Development

No branches or pull requests

2 participants