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

UI/kv codemirror diff #13000

Merged
merged 27 commits into from
Dec 1, 2021
Merged

UI/kv codemirror diff #13000

merged 27 commits into from
Dec 1, 2021

Conversation

Monkeychip
Copy link
Contributor

@Monkeychip Monkeychip commented Nov 1, 2021

Adding a Diff view for the KV2 secrets engine.

See video for the diff view. It only shows if you have read access and more than 1 version of a secret. If you have deleted or destroyed a secret it will show null on page load and then not let you select that secret version. The icons in the version dropdown help you understand the secret's status.

To add this feature I added a new route and new component. I had played with extending the json-editor component that we currently use with codemirror, but that proved too messy. Additional we use a different library (jsonDiffPatch, used by another Hashicorp product) instead of codemirror because of dependency loading issues.

Screen.Recording.2021-11-29.at.03.12.39.PM.mp4

@vercel vercel bot temporarily deployed to Preview – vault November 3, 2021 16:15 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook November 3, 2021 16:25 Inactive
@vercel vercel bot temporarily deployed to Preview – vault November 18, 2021 01:53 Inactive
@Monkeychip Monkeychip marked this pull request as ready for review November 29, 2021 22:28
@@ -1,4 +1,21 @@
import Component from '@ember/component';
Copy link
Contributor Author

@Monkeychip Monkeychip Nov 29, 2021

Choose a reason for hiding this comment

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

only change is glimmerized this component. I was originally here thinking I'd modify this component. In the end I didn't use this component, but played boy scout and "left it better than I found it."

color: $ui-gray-010;
}
}

Copy link
Contributor Author

@Monkeychip Monkeychip Nov 29, 2021

Choose a reason for hiding this comment

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

over-riding the jsondiffpatch colors. We wanted a specific red and green to match the reds and greens we use in the app. confirmed colors with design.

@@ -1,32 +1,34 @@
{{#if showToolbar }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this diff looks strange, but I think when this was last worked on the developer had their tab settings at 4. I changed the file tab settings to 2 to match the code base and this was the resulting diff. If you're nervous checkout the file in the branch and double check.

Copy link
Contributor

Choose a reason for hiding this comment

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

I appreciate all of your notes! 😍

@vercel vercel bot temporarily deployed to Preview – vault November 29, 2021 22:33 Inactive
@@ -1,7 +1,7 @@
import { isPresent, fillable, clickable } from 'ember-cli-page-object';

export default {
showsJsonViewer: isPresent('[data-test-json-viewer]'),
showsJsonViewer: isPresent('[data-test-component="json-editor"]'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

because of changing the component to glimmer, this was wrapped around the component, but now that it's glimmer that doesn't work.

@@ -11,7 +11,6 @@
<div class="control-group-success {{if unwrapData 'is-editor'}}">
<div class="has-copy-button">
<JsonEditor
data-test-json-viewer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing because this component has been changed to glimmer.

Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

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

Well done! This is awesome :)

@Monkeychip Monkeychip merged commit 720db8e into main Dec 1, 2021
@Monkeychip Monkeychip deleted the ui/kv-codemirror-diff branch December 1, 2021 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants