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 args to tree #1112

Merged
merged 20 commits into from
Feb 5, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ module.exports = {
files: [
'.ember-cli.js',
'.eslintrc.js',
'.prettierrc.js',
'.stylelintrc.js',
'.template-lintrc.js',
'ember-cli-build.js',
Expand Down
14 changes: 14 additions & 0 deletions .prettierrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';

module.exports = {
singleQuote: true,
overrides: [
{
files: '**/*.hbs',
options: {
parser: 'glimmer',
singleQuote: false
}
}
]
};
8 changes: 8 additions & 0 deletions app/controllers/component-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,18 @@ class RenderItem {
this.renderNode = renderNode;
}

get args() {
return this.renderNode.args;
}

get id() {
return this.renderNode.id;
}

get isCurlyInvocation() {
return this.renderNode && this.renderNode.args && this.renderNode.args.positional;
}

RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
get isOutlet() {
return this.renderNode.type === 'outlet';
}
Expand Down
20 changes: 20 additions & 0 deletions app/helpers/component-argument.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { helper } from '@ember/component/helper';

/**
* Determine the type of the component argument for display
*
* @method componentArgumentDisplay
* @param {*} argument
* @return {*} The argument with the correct type for display
*/
export function componentArgumentDisplay([argument]) {
if (typeof argument === 'string') {
return JSON.stringify(argument).replace(/^\"|\"$/g, '');
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
} else if (typeof argument === 'object' && argument !== null) {
return '...';
}

return String(argument);
}

export default helper(componentArgumentDisplay);
7 changes: 7 additions & 0 deletions app/helpers/is-string.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { helper } from '@ember/component/helper';

export function isString([str]) {
return typeof str === 'string';
}

export default helper(isString);
8 changes: 6 additions & 2 deletions app/styles/colors.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@
--transparent: transparent;
--focus: #3F81EE;
--focus-text: var(--white);
--component-text: #7c027c;
--component-name: #1171E8;
--component-brackets-selected: rgba(255, 255, 255, 0.4);
--component-attr: #DB00A9;
--component-highlighted-bg: rgba(0, 0, 0, 0.05);
--pill-bg: rgba(0, 0, 0, 0.1);
--pill-bg-active: rgba(255, 255, 255, 0.3);
--warning-text: #735c0f;
Expand Down Expand Up @@ -74,8 +76,10 @@
--transparent: transparent;
--focus: #2270EC;
--focus-text: var(--white);
--component-text: #FF8BC9;
--component-name: #77BEFF;
--component-brackets-selected: rgba(255, 255, 255, 0.4);
--component-attr: #FE7AE9;
--component-highlighted-bg: rgba(255, 255, 255, 0.075);
--pill-bg: rgba(255, 255, 255, 0.2);
--pill-bg-active: rgba(255, 255, 255, 0.3);
--warning-text: #8ca3f0;
Expand Down
72 changes: 59 additions & 13 deletions app/styles/component_tree.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@
margin: 0 3px;
min-height: 22px;
position: relative;

&:hover {
background-color: var(--base05);

.component-tree-item__action {
opacity: 1;
}

.component-tree-item__action.disabled {
opacity: 0.65;
}
}
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
}

.component-tree-item__expand {
Expand Down Expand Up @@ -45,36 +57,70 @@
.component-tree-item__tag {
cursor: default;
display: flex;
white-space: nowrap;

.component-name {
color: var(--component-name);
}

.arg-token {
display: flex;
margin-left: 0.5rem;
}

.bracket-token {
color: var(--base09);
}

.at-token,
.key-token {
color: var(--component-attr);
}

.value-token {
color: var(--spec03);
}
}

.component-tree-item--has-instance .component-tree-item__tag {
cursor: pointer;
}

.component-tree-item--selected .component-tree-item__tag {
.component-name,
.at-token,
.bracket-token,
.key-token,
.value-token {
color: var(--focus-text);
}
}

.component-tree-item__bracket:before,
.component-tree-item__bracket:after,
.component-tree-item-classic__bracket:before,
.component-tree-item-classic__bracket:after {
color: var(--base09);
}

.component-tree-item__bracket:before {
color: var(--base07);
content: '<';
}

.component-tree-item__bracket:after {
color: var(--base07);
content: '>';
}

.component-tree-item__bracket.component-tree-item__bracket--self-closing:after {
content: '/>';
}

.component-tree-item:hover {
background-color: var(--spec06);

.component-tree-item__action {
opacity: 1;
}
.component-tree-item-classic__bracket:before {
content: '{{';
}

.component-tree-item__action.disabled {
opacity: 0.65;
}
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
.component-tree-item-classic__bracket:after {
content: '}}';
}

/**
Expand All @@ -83,7 +129,7 @@
*/

.component-tree-item.component-tree-item--highlighted {
background-color: var(--spec06);
background-color: var(--component-highlighted-bg);
border-radius: 0;
}

Expand Down Expand Up @@ -122,5 +168,5 @@
}

.component-tree-item--component {
color: var(--component-text);
color: var(--base09);
}
80 changes: 75 additions & 5 deletions app/templates/components/component-tree-item.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
{{if @item.isComponent "component-tree-item--component"}}
{{if @item.hasInstance "component-tree-item--has-instance"}}
{{if @item.isSelected "component-tree-item--selected"}}
{{if @item.isHighlighted "component-tree-item--highlighted"}}
Copy link
Member

Choose a reason for hiding this comment

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

why is this removed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I could tell that was being used for the hover state. See: https://github.com/emberjs/ember-inspector/pull/1129/files#diff-cbd2aa0d940eef1f44430e6ca030fbd9L8

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is the original PR: #984

Copy link
Member

Choose a reason for hiding this comment

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

I see. Yeah that changed in Octane. It is now used to highlight the "currently previewing component". IMO the previous behavior is not that useful, since you can already see the parent/child relationship from the nesting.

There are two ways to "preview" a component, by hovering the rows in the component tree or by using the "live inspection" feature and hover over the DOM elements on the page:

highlight

Using CSS hover state only solves the first part and I think we still need this for the latter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oooooh. I see. I'll have to revert that.

"
{{if @item.isHighlighted "component-tree-item--highlighted"}}"
{{on "click" @item.toggleInspection}}
{{on "mouseenter" @item.showPreview}}
{{on "mouseleave" @item.hidePreview}}
Expand All @@ -21,10 +20,81 @@
{{/if}}

<code
class="component-tree-item__tag {{if @item.isComponent "component-tree-item__bracket"}} {{if (not @item.hasChildren) "component-tree-item__bracket--self-closing"}}"
class="component-tree-item__tag
{{if
(and (not @item.isCurlyInvocation) @item.isComponent)
"component-tree-item__bracket"
}}
{{if
(and (not @item.isCurlyInvocation) (not @item.hasChildren) @item.isComponent)
"component-tree-item__bracket--self-closing"
}}
{{if
(and @item.isCurlyInvocation @item.isComponent)
"component-tree-item-classic__bracket"
}}"
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
>
{{#if @item.isComponent}}
{{classify @item.name}}
{{#if @item.args.positional}}
<span class="component-name">
{{@item.name}}
</span>

{{#each @item.args.positional as |value|}}
<div class="arg-token">
<span class="value-token">
{{component-argument value}}
</span>
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
</div>
{{/each}}

{{#each-in @item.args.named as |name value|}}
<div class="arg-token">
<span class="key-token">
{{name}}
</span>
=
<span class="value-token">
{{component-argument value}}
</span>
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
</div>
{{/each-in}}
{{else}}
<span class="component-name">
{{classify @item.name}}
</span>

{{#each-in @item.args.named as |name value|}}
<div class="arg-token">
{{#if (is-string value)}}
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
<span class="at-token">
@
</span>
<span class="key-token">
{{name}}
</span>
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
="
<span class="value-token">
{{component-argument value}}
</span>
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
"
{{else}}
<span class="at-token">
@
</span>
RobbieTheWagner marked this conversation as resolved.
Show resolved Hide resolved
<span class="key-token">
{{name}}
</span>
=
<span class="bracket-token">\{{</span>
<span class="value-token">
{{component-argument value}}
</span>
<span class="bracket-token">}}</span>
{{/if}}
</div>
{{/each-in}}
{{/if}}
{{else if @item.isOutlet}}
\{{outlet "{{@item.name}}"}}
{{else if @item.isEngine}}
Expand All @@ -51,4 +121,4 @@
{{svg-jar "code-line" width="20px" height="20px"}}
</button>
{{/if}}
</div>
</div>
3 changes: 1 addition & 2 deletions lib/ui/addon/styles/_list.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
}

.list__content {
overflow: hidden;
overflow-y: scroll;
overflow: scroll;
transform: translateZ(0);
}

Expand Down
7 changes: 5 additions & 2 deletions lib/ui/addon/styles/_split.scss
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
flex: auto;
flex-direction: column;
position: relative;
overflow: hidden;
}

.split__panel__hd,
Expand Down Expand Up @@ -51,7 +52,9 @@
.split__panel__bd {
background: var(--base00);
flex: auto;
overflow: auto;
overflow: scroll;
max-width: 100%;
width: auto;
}

.split__panel__ft {
Expand Down Expand Up @@ -83,7 +86,7 @@

/* Fix main list-view scrolling */
.split--main > .split__panel > .split__panel__bd {
overflow-y: hidden;
overflow: hidden;
}

.split--main > .split__panel--sidebar-1 > .split__panel__bd {
Expand Down