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.

Sign up for GitHub

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: preparation for HDS adoption to replace <Modal> #23353

Merged
merged 3 commits into from
Sep 27, 2023
Merged

Ui: preparation for HDS adoption to replace <Modal> #23353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25dc866
add autoRefresh to codemirror
hellobontempo Sep 26, 2023
7fa0c56
fix codemirror arg
hellobontempo Sep 26, 2023
9c05759
move scroll to body to accommodate hds mdoals
hellobontempo Sep 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ui/app/styles/core/element-styling.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ html {
-moz-osx-font-smoothing: grayscale;
-webkit-font-smoothing: antialiased;
min-width: 300px;
overflow-x: hidden;
overflow-y: scroll;
text-rendering: optimizeLegibility;
text-size-adjust: 100%;
}
Expand Down Expand Up @@ -124,6 +122,8 @@ pre code {
body {
font-size: $size-6;
line-height: 1.5;
overflow-x: hidden;
overflow-y: scroll;
}

a {
Expand Down
2 changes: 2 additions & 0 deletions ui/lib/core/addon/components/json-editor.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
{{/if}}
<div class="toolbar-separator"></div>
<Hds::Copy::Button
@container={{@container}}
@text="Copy"
@isIconOnly={{true}}
@textToCopy={{@value}}
Expand All @@ -52,6 +53,7 @@
onSetup=this.onSetup
onUpdate=this.onUpdate
onFocus=this.onFocus
autoRefresh=(if @container true false)
}}
class={{if @readOnly "readonly-codemirror"}}
data-test-component="code-mirror-modifier"
Expand Down
4 changes: 4 additions & 0 deletions ui/lib/core/addon/components/json-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ import { action } from '@ember/object';
* @param {String} [value] - Value within the display. Generally, a json string.
* @param {String} [viewportMargin] - Size of viewport. Often set to "Infinity" to load/show all text regardless of length.
* @param {string} [example] - Example to show when value is null -- when example is provided a restore action will render in the toolbar to clear the current value and show the example after input
* * REQUIRED if rendering within a modal *
* @container gives context for the <Hd::Copy::Button> and sets autoRefresh=true so JsonEditor renders content (without this property @value only renders if editor is focused)
* @param {string} [container] - Selector string or element object of containing element, set the focused element as the container value. This is for the Hds::Copy::Button and to set autoRefresh=true so content renders https://hds-website-hashicorp.vercel.app/components/copy/button?tab=code
*
Comment on lines +30 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice thanks for documenting this!

*/

export default class JsonEditorComponent extends Component {
Expand Down
2 changes: 2 additions & 0 deletions ui/lib/core/addon/modifiers/code-mirror.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Modifier from 'ember-modifier';

import 'codemirror/addon/edit/matchbrackets';
import 'codemirror/addon/selection/active-line';
import 'codemirror/addon/display/autorefresh';
Copy link
Contributor Author

@hellobontempo hellobontempo Sep 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

[docs]

When a modal renders the JsonEditor (ex: with the sample policy template) the content of JsonEditor doesn't render unless the element is focused. Previously we would wait to render the component by wrapping the element in an additional conditional, but this approach no longer works. Now codemirror will autorefresh if passed a @container arg

https://github.com/hashicorp/vault/blob/main/ui/app/templates/components/oidc/scope-form.hbs#L100-L104

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it would be necessary in scenarios other than in a modal where the container value is passed to set the autoRefresh property? I'm guessing no since we haven't run into this until now and we could easily add a dedicated arg for autoRefresh in the future if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - that was my thinking. For now, it seemed okay to have autoRefresh rely on container. But since container could refer to a flyout, dropdown, or maybe even a card - we may want to separate the args in the future!

import 'codemirror/addon/lint/lint.js';
import 'codemirror/addon/lint/json-lint.js';
// right now we only use the ruby and javascript, if you use another mode you'll need to import it.
Expand Down Expand Up @@ -62,6 +63,7 @@ export default class CodeMirrorModifier extends Modifier {
theme: namedArgs.theme || 'hashi',
value: namedArgs.content || '',
viewportMargin: namedArgs.viewportMargin || '',
autoRefresh: namedArgs.autoRefresh,
});

editor.on('change', bind(this, this._onChange, namedArgs));
Expand Down