Skip to content

fix: vertical rule doesn't render #476

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

Merged
jianliao merged 4 commits into from
Jan 22, 2020
Merged

fix: vertical rule doesn't render #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
869c5b7
fix: make vertical Rule's height a variable, fixes #455
lazd Jan 7, 2020
b19a9b3
fix: update Rule token names, related #425
lazd Jan 14, 2020
ed00cbe
docs: use ActionButton instead of Tool in Rule examples
lazd Jan 22, 2020
ea7b85f
feat: support vertical Rules inside of ButtonGroup, related to #455
lazd Jan 14, 2020
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
5 changes: 5 additions & 0 deletions components/buttongroup/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ governing permissions and limitations under the License.
flex-shrink: 0;
}

> .spectrum-Rule--vertical {
height: auto;
align-self: stretch;
}
Copy link
Member Author

@lazd lazd Jan 22, 2020

Choose a reason for hiding this comment

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

This makes it so Rules inside of ButtonGroups look nice.


.spectrum-Button + .spectrum-Button {
margin-left: var(--spectrum-buttongroup-button-gap-x);
}
Expand Down
31 changes: 19 additions & 12 deletions components/rule/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,47 +12,54 @@ governing permissions and limitations under the License.

@import '../commons/index.css';

:root {
--spectrum-divider-vertical-height: 100%;
Copy link
Member Author

@lazd lazd Jan 22, 2020

Choose a reason for hiding this comment

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

Defining height as a variable fixes #455


/* Work around DNA misspelling */
--spectrum-divider-small-vertical-width: var(--spectrum-divider-small-vertical-width, var(--spectrum-divider-small-veritcal-width));
Copy link
Member Author

@lazd lazd Jan 22, 2020

Choose a reason for hiding this comment

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

The current version of DNA has this token, but it's spelled wrong var(--spectrum-divider-small-veritcal-width). This works around that issue, and should work properly once the misspelled token is removed.

}

.spectrum-Rule {
width: 100%;

/* Show the overflow for hr in Edge and IE. */
overflow: visible;

border: none;
border-width: var(--spectrum-rule-medium-height);
border-radius: var(--spectrum-rule-medium-height);
border-width: var(--spectrum-divider-medium-height);
border-radius: var(--spectrum-divider-medium-height);
Copy link
Member Author

@lazd lazd Jan 22, 2020

Choose a reason for hiding this comment

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

All tokens renamed from -rule to -divider since the component was renamed in DNA. The previous tokens are still there, but are missing the new vertical-width tokens.

}

.spectrum-Rule--large {
height: var(--spectrum-rule-large-height);
height: var(--spectrum-divider-large-height);

border-radius: var(--spectrum-rule-large-border-radius);
border-radius: var(--spectrum-divider-large-border-radius);
}

.spectrum-Rule--medium {
height: var(--spectrum-rule-medium-height);
height: var(--spectrum-divider-medium-height);

border-radius: var(--spectrum-rule-medium-border-radius);
border-radius: var(--spectrum-divider-medium-border-radius);
}

.spectrum-Rule--small {
height: var(--spectrum-rule-small-height);
height: var(--spectrum-divider-small-height);

border-radius: var(--spectrum-rule-small-border-radius);
border-radius: var(--spectrum-divider-small-border-radius);
}

.spectrum-Rule--vertical {
height: 100%;
height: var(--spectrum-divider-vertical-height);

&.spectrum-Rule--large {
width: var(--spectrum-rule-large-height);
width: var(--spectrum-divider-large-vertical-width);
}

&.spectrum-Rule--medium {
width: var(--spectrum-rule-medium-height);
width: var(--spectrum-divider-medium-vertical-width);
}

&.spectrum-Rule--small {
width: var(--spectrum-rule-small-height);
width: var(--spectrum-divider-small-vertical-width);
}
}
55 changes: 25 additions & 30 deletions components/rule/metadata/rule.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,41 +22,36 @@ examples:
<p class="spectrum-Body">Divide like-elements (tables, tool groups, elements within a panel, etc.)</p>
- id: rule-vertical-small
name: 'Vertical, Small'
description: |
When a vertical Rule is used inside of a flex container, use `align-self: stretch; height: auto` on the Rule.
ButtonGroup automatically applies these styles to vertical Rules.
markup: |
<div style="display: flex; flex-direction: row; height: 32px">
<div class="spectrum-ButtonGroup">
<button class="spectrum-Tool">
<svg class="spectrum-Icon spectrum-Icon--sizeS" focusable="false" aria-hidden="true" aria-label="Properties">
<use xlink:href="#spectrum-icon-18-Properties" />
</svg>
</button>
</div>
<div class="spectrum-ButtonGroup">
<button class="spectrum-ActionButton spectrum-ActionButton--quiet">
<svg class="spectrum-Icon spectrum-Icon--sizeS" focusable="false" aria-hidden="true" aria-label="Properties">
<use xlink:href="#spectrum-icon-18-Properties" />
</svg>
</button>
<div class="spectrum-Rule spectrum-Rule--small spectrum-Rule--vertical"></div>
<div class="spectrum-ButtonGroup">
<button class="spectrum-Tool">
<svg class="spectrum-Icon spectrum-Icon--sizeS" focusable="false" aria-hidden="true" aria-label="Select">
<use xlink:href="#spectrum-icon-18-Select" />
</svg>
</button>
</div>
<button class="spectrum-ActionButton spectrum-ActionButton--quiet">
Copy link
Member Author

@lazd lazd Jan 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Switched to ActionButton because Tool is deprecated. They are visually identical.

<svg class="spectrum-Icon spectrum-Icon--sizeS" focusable="false" aria-hidden="true" aria-label="Select">
<use xlink:href="#spectrum-icon-18-Select" />
</svg>
</button>
</div>
- id: rule-vertical-medium
name: 'Vertical, Medium'
markup: |
<div style="display: flex; flex-direction: row; height: 32px">
<div class="spectrum-ButtonGroup">
<button class="spectrum-Tool">
<svg class="spectrum-Icon spectrum-Icon--sizeS" focusable="false" aria-hidden="true" aria-label="Properties">
<use xlink:href="#spectrum-icon-18-Properties" />
</svg>
</button>
</div>
<div class="spectrum-ButtonGroup">
<button class="spectrum-ActionButton spectrum-ActionButton--quiet">
<svg class="spectrum-Icon spectrum-Icon--sizeS" focusable="false" aria-hidden="true" aria-label="Properties">
<use xlink:href="#spectrum-icon-18-Properties" />
</svg>
</button>
<div class="spectrum-Rule spectrum-Rule--medium spectrum-Rule--vertical"></div>
<div class="spectrum-ButtonGroup">
<button class="spectrum-Tool">
<svg class="spectrum-Icon spectrum-Icon--sizeS" focusable="false" aria-hidden="true" aria-label="Select">
<use xlink:href="#spectrum-icon-18-Select" />
</svg>
</button>
</div>
<button class="spectrum-ActionButton spectrum-ActionButton--quiet">
<svg class="spectrum-Icon spectrum-Icon--sizeS" focusable="false" aria-hidden="true" aria-label="Select">
<use xlink:href="#spectrum-icon-18-Select" />
</svg>
</button>
</div>
1 change: 1 addition & 0 deletions components/rule/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
},
"devDependencies": {
"@spectrum-css/button": "^2.0.3",
"@spectrum-css/buttongroup": "^2.0.3",
Copy link
Member Author

@lazd lazd Jan 22, 2020

Choose a reason for hiding this comment

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

This dependency was missing before.

"@spectrum-css/component-builder": "^1.0.2",
"@spectrum-css/vars": "^1.0.0-alpha.0",
"gulp": "^4.0.0"
Expand Down