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

Jump to bottom

ui: code-editor-styling #11474

Merged
merged 21 commits into from
Nov 12, 2021
Merged

ui: code-editor-styling #11474

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
7dfbc6e
WIP: code-editor-styling
deblasis Nov 2, 2021
ba53c6a
changelog
deblasis Nov 2, 2021
f2cfacd
fix: toolbar title & link for policies
deblasis Nov 3, 2021
5035cfd
fix: width fix + dropdown styling
deblasis Nov 3, 2021
c9444da
fix: toolbar styling variables
deblasis Nov 3, 2021
363e432
fix: ACL rules docs link on `fieldsets-legacy`
deblasis Nov 3, 2021
c72b6fc
WIP: code-editor-styling
deblasis Nov 2, 2021
a662d5f
changelog
deblasis Nov 2, 2021
76ac2a8
fix: toolbar title & link for policies
deblasis Nov 3, 2021
e97e76f
fix: width fix + dropdown styling
deblasis Nov 3, 2021
614cc51
fix: toolbar styling variables
deblasis Nov 3, 2021
d241512
fix: ACL rules docs link on `fieldsets-legacy`
deblasis Nov 3, 2021
8bada55
Merge branch 'ui/bugfix/code-editor-styling' of github.com:deblasis/c…
deblasis Nov 4, 2021
5f3bee3
Merge branch 'hashicorp:main' into ui/bugfix/code-editor-styling
deblasis Nov 4, 2021
bb014ff
Merge branch 'ui/bugfix/code-editor-styling' of github.com:deblasis/c…
deblasis Nov 4, 2021
bca9ad0
refactoring: using named blocks
deblasis Nov 4, 2021
3a3d0ef
fix: removed unnecessary property
deblasis Nov 4, 2021
b149aff
fix: themeable black/white in dropdown
deblasis Nov 4, 2021
24c43a2
fix: use variables for border
deblasis Nov 4, 2021
ed20fb7
ui: CodeEditor refactoring
deblasis Nov 4, 2021
f160233
Update ui/packages/consul-ui/app/components/code-editor/README.mdx
deblasis Nov 12, 2021
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
6 changes: 6 additions & 0 deletions .changelog/11474.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
```release-note:bug
ui: code editor styling (layout consistency + wide screen support)
```
```release-note:improvement
ui: added copy to clipboard button in code editor toolbars
```
31 changes: 21 additions & 10 deletions ui/packages/consul-ui/app/components/code-editor/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
<Toolbar>
<label class="title">
{{#if (has-block "title")}}
{{yield to="title"}}
{{/if}}
</label>
<div class="actions">
{{#if (and (not readonly) (not syntax))}}
<PowerSelect
@onChange={{action "change"}}
@selected={{mode}}
@searchEnabled={{false}}
@options={{modes}} as |mode|>
{{mode.name}}
</PowerSelect>
{{/if}}
<div class="toolbar-separator"></div>
<CopyButton @value={{value}} @name="value" />
</div>
</Toolbar>
<IvyCodemirror @value={{value}} @name={{name}} @class={{class}} @options={{options}} @valueUpdated={{action onkeyup}} />
<pre><code>{{yield}}</code></pre>
{{#if (and (not readonly) (not syntax))}}
<PowerSelect
@onChange={{action "change"}}
@selected={{mode}}
@searchEnabled={{false}}
@options={{modes}} as |mode|>
{{mode.name}}
</PowerSelect>
{{/if}}
<pre><code>{{#if (has-block "default")}}{{yield to="default"}}{{else}}{{value}}{{/if}}</code></pre>
119 changes: 119 additions & 0 deletions ui/packages/consul-ui/app/components/code-editor/layout.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
border: 10px;
overflow: hidden;
position: relative;
clear: both;
}
%code-editor .ember-power-select-trigger {
@extend %code-editor-syntax-select;
Expand Down Expand Up @@ -32,3 +33,121 @@
%code-editor > pre {
display: none;
}

%code-editor .ember-basic-dropdown-trigger {
background-color: var(--tone-gray-900);
color: var(--black);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use --tone-gray-000 for this please? (instead of --black)

All of the --tones are themeable --tone-gray-000 is black (in light mode) and --tone-gray-999 is white. We have this "quick dark mode" thing which just reverses the gray tones (so 000 = white and 999 = black), which gets us almost to dark mode or high contrast mode really easily.

So glad you did it like this though (but I'd like you to change) as it means I know I need to add this to our color docs 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I must be using the dark mode because
image
comes from this:
image
I just pushed this specific change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh sorry wrote that wrong 🤦 its the other way around. Have a look here https://github.com/hashicorp/consul/blob/main/ui/packages/consul-ui/app/styles/base/color/ui/themes/light.scss#L1

border-radius: 2px;
border: 1px solid;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not super important but we have some more props for these if you could use those

https://github.com/hashicorp/consul/blob/main/ui/packages/consul-ui/app/styles/base/decoration/base-variables.scss#L4-L10

Documentation for all this coming soon!

border-color: var(--tone-gray-700);
margin: 0 8px;
width: 120px;
height: 32px;
display: flex;
align-items: center;
flex-direction: row;
}
Copy link
Contributor

@johncowen johncowen Nov 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

There are a few missing rgb here. I dunno if you noticed but in the ColorSwatch docs things I linked to, if you click on the swatch it copies the value (inc. rgb) to the clipboard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I noticed but I was getting the wrong colours because of the dark mode I guess so I ended up writing manually (the wrong thing).

Also I noticed that there were some inconsistencies in that file already, see

consul/ui/packages/consul-ui/app/components/code-editor/layout.scss

Line 18 in 2596135

background-color: rgb(var(--black));
and

consul/ui/packages/consul-ui/app/components/code-editor/layout.scss

Line 28 in 2596135

background-color: var(--black);

Copy link
Contributor

@johncowen johncowen Nov 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yup only --tones should use the rgb syntax. Any colours without --tone should pretty much be ignored/not used.


.toolbar {
background-color: rgb(var(--tone-gray-050));
border: 1px solid rgb(var(--tone-gray-100));
border-bottom-color: rgb(var(--tone-gray-600));
border-top-color: rgb(var(--tone-gray-400));
position: relative;
margin-top: 4px;

&::after {
background: linear-gradient(to right, rgb(var(--tone-gray-600)), rgba(var(--tone-gray-600), 0));
bottom: 0;
content: '';
position: absolute;
right: 0;
top: 0;
width: 4px;
z-index: 2;
}

.input,
.select {
min-width: 190px;
}
}

.toolbar-label {
padding: 8px;
color: rgb(var(--tone-gray-800));
}

.toolbar-scroller {
align-items: center;
display: flex;
height: 44px;
justify-content: space-between;
overflow-x: auto;
width: 100%;

&::-webkit-scrollbar {
border-bottom: 1px solid rgb(var(--tone-gray-400));
height: 4px;
}

&::-webkit-scrollbar-thumb {
background: rgb(var(--tone-gray-400));
border-radius: 4px;
}
}

.toolbar-filters,
.toolbar-actions {
align-items: center;
display: flex;
flex: 1;
white-space: nowrap;
justify-content: space-between;

.title {
color: rgb(var(--tone-gray-900));
display: inline-block;
font-size: 14px;
font-weight: 700;
padding: 0 8px;
}
.actions {
display: flex;
flex-direction: row;
margin: 0 10px;
align-items: center;

.copy-button {
margin-left: 10px;
}
}
}

.toolbar-link {
border: 0;
color: var(--black);
transition: background-color 150ms;

&:hover {
background-color: rgb(var(--tone-gray-050));
border: 0;
color: rgb(var(--tone-blue-500));
}

&:active {
box-shadow: none;
}

&.popup-menu-trigger {
height: 2.5rem;
padding: 0.5rem 0.857rem;
}
}

.toolbar-separator {
border-right: 1px solid rgb(var(--tone-gray-300));
height: 32px;
margin: 0 4px;
width: 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels like there a lot here, so I'm gonna take a deeper look, so up to you if you want to make changes now or wait til then. but generally:

  1. If you could use that CSS props for decoration things like borders etc that I linked to above that'd be great.
  2. Use a tone instead of black (as above)
  3. All in all this will need splitting into layout and skin files - but we can leave that until absolute last thing as its just cleanup/consistency - I'd rather get everything finished and then do a final clean up, but up to you if you wanted to do it now instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree and I like the splitting, right, since I am going to refactor the Toolbar anyway now that I know how to do it ;)
I will let you know when this is ready for review again so that I don't waste too much of your time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds great 👍

2 changes: 1 addition & 1 deletion ui/packages/consul-ui/app/components/code-editor/skin.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ $syntax-dark-gray: #535f73;
--syntax-yellow: rgb(var(--tone-yellow-500));
}
.CodeMirror {
max-width: 1150px;
max-width: 1260px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh dear 😆 it was just this 🤣 . Surprising nobody has tweaked this before now 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, right? It happens, it's always a single line of code that is holding things together or that breaks everything :)

min-height: 300px;
height: auto;
/* adds some space at the bottom of the editor for when a horizonal-scroll has appeared */
Expand Down
7 changes: 5 additions & 2 deletions ui/packages/consul-ui/app/components/consul/kv/form/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,19 @@
<span>Code</span>
</label>
</div>

<label for="" class="type-text{{if api.data.error.Value ' has-error'}}">
<span>Value</span>
{{#if json}}
<CodeEditor
@name="value"
@readonly={{or disabld api.disabled}}
@value={{atob api.data.Value}}
@onkeyup={{action api.change "value"}}
/>
>
<:title>Value</:title>
</CodeEditor>
{{else}}
<span>Value</span>
<textarea
{{disabled (or disabld api.disabled)}}
autofocus={{not api.isCreate}}
Expand Down
35 changes: 26 additions & 9 deletions ui/packages/consul-ui/app/components/policy-form/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,34 +37,51 @@
{{/if}}
</label>
<label class="type-text" data-test-rules>
<span>Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a></span>
{{#if (eq item.template 'service-identity')}}
<CodeEditor
@readonly={{true}}
@name={{concat name "[Rules]"}}
@syntax="hcl"
@oninput={{action "change" (concat name "[Rules]")}}
><Consul::ServiceIdentity::Template
@nspace={{nspace}}
@name={{item.Name}}
/></CodeEditor>
>
<:title>
Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a>
</:title>
<:default>
<Consul::ServiceIdentity::Template
@nspace={{nspace}}
@name={{item.Name}}
/>
</:default>
</CodeEditor>
{{else if (eq item.template 'node-identity')}}
<CodeEditor
@readonly={{true}}
@name={{concat name "[Rules]"}}
@syntax="hcl"
@oninput={{action "change" (concat name "[Rules]")}}
><Consul::NodeIdentity::Template
@name={{item.Name}}
/></CodeEditor>
>
<:title>
Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a>
</:title>
<:default>
<Consul::NodeIdentity::Template
@name={{item.Name}}
/>
</:default>
</CodeEditor>
{{else}}
<CodeEditor
@syntax="hcl"
@class={{if item.error.Rules "error"}}
@name={{concat name "[Rules]"}}
@value={{item.Rules}}
@onkeyup={{action "change" (concat name "[Rules]")}}
/>
>
<:title>
Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a>
</:title>
</CodeEditor>
{{#if item.error.Rules}}
<strong>{{item.error.Rules.validation}}</strong>
{{/if}}
Expand Down
36 changes: 27 additions & 9 deletions ui/packages/consul-ui/app/components/policy-selector/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,28 +114,46 @@
</dl>
{{/if}}
<label class="type-text">
<span>Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a></span>
{{#if (eq item.template 'service-identity')}}
<CodeEditor
@syntax="hcl"
@readonly={{true}}
><Consul::ServiceIdentity::Template
@nspace={{nspace}}
@name={{item.Name}}
/></CodeEditor>
>
<:title>
Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a>
</:title>
<:default>
<Consul::ServiceIdentity::Template
@nspace={{nspace}}
@name={{item.Name}}
/>
</:default>
</CodeEditor>
{{else if (eq item.template 'node-identity')}}
<CodeEditor
@syntax="hcl"
@readonly={{true}}
><Consul::NodeIdentity::Template
@name={{item.Name}}
/></CodeEditor>
>
<:title>
Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a>
</:title>
<:default>
<Consul::NodeIdentity::Template
@name={{item.Name}}
/>
</:default>
</CodeEditor>
{{else}}
<CodeEditor
@toolbar={{ hash title="Rules" helplink=helplink }}
@syntax="hcl"
@readonly={{true}}
@value={{or loadedItem.Rules item.Rules}}
/>
>
<:title>
Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a>
</:title>
</CodeEditor>
{{/if}}
</label>
{{#if (not disabled)}}
Expand Down
7 changes: 7 additions & 0 deletions ui/packages/consul-ui/app/components/toolbar/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<nav class="toolbar">
<nav class="toolbar-actions">
<div class="toolbar-scroller">
{{yield}}
</div>
</nav>
</nav>
Copy link
Contributor

@johncowen johncowen Nov 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

afaik nav should only be used for site / app main nav (or sub nav or whatever) but its generally a way to navigate around the site / app rather than for a toolbar thing. I'd just stick with div here unless you can suggest something else? (I'd be concerned about suggesting menu as I vaguely remember browser difference/complications from a while back but I've no idea if thats still relevant, so unless you know more I'd maybe err on the side of caution with a div)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I stole this from Vault and I played ignorantly to be honest but yeah, it is especially weird because we have nested nav too. I'll change that.

8 changes: 6 additions & 2 deletions ui/packages/consul-ui/app/templates/dc/acls/tokens/-fieldsets-legacy.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@
</div>
{{/if}}
<label class="type-text">
<span>Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a></span>
<CodeEditor @class={{if item.error.Rules "error"}} @name="Rules" @syntax="hcl" @value={{item.Rules}} @onkeyup={{action "change" "Rules"}} />
<CodeEditor
@class={{if item.error.Rules "error"}} @name="Rules" @syntax="hcl" @value={{item.Rules}} @onkeyup={{action "change" "Rules"}}>
<:title>
Rules <a href="{{env 'CONSUL_DOCS_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a>
</:title>
</CodeEditor>
</label>
{{#if create }}
<label class="type-text">
Expand Down