Unverified Commit c7d2ef4e authored by Lyza Gardner's avatar Lyza Gardner Committed by GitHub

Apply design conventions to `AnnotationShareControl` (#2304)

- Adjust a few related mixins
- Adjust tests; make them less brittle on CSS classes
- Fix arrow alignment for narrow viewports
parent 69187d96
...@@ -101,7 +101,7 @@ function AnnotationShareControl({ ...@@ -101,7 +101,7 @@ function AnnotationShareControl({
</div> </div>
</div> </div>
<div className="annotation-share-panel__content"> <div className="annotation-share-panel__content">
<div className="annotation-share-panel__input u-layout-row"> <div className="u-layout-row">
<input <input
aria-label="Use this URL to share this annotation" aria-label="Use this URL to share this annotation"
className="annotation-share-panel__form-input" className="annotation-share-panel__form-input"
......
...@@ -29,7 +29,7 @@ function AnnotationShareInfo({ annotation }) { ...@@ -29,7 +29,7 @@ function AnnotationShareInfo({ annotation }) {
<div className="annotation-share-info u-layout-row--align-baseline"> <div className="annotation-share-info u-layout-row--align-baseline">
{linkToGroup && ( {linkToGroup && (
<a <a
className="annotation-share-info__group" className="u-layout-row--align-baseline u-color-text--muted"
href={group.links.html} href={group.links.html}
target="_blank" target="_blank"
rel="noopener noreferrer" rel="noopener noreferrer"
...@@ -45,7 +45,7 @@ function AnnotationShareInfo({ annotation }) { ...@@ -45,7 +45,7 @@ function AnnotationShareInfo({ annotation }) {
</a> </a>
)} )}
{annotationIsPrivate && !linkToGroup && ( {annotationIsPrivate && !linkToGroup && (
<span className="annotation-share-info__private"> <span className="u-layout-row--align-baseline u-color-text--muted">
<span className="annotation-share-info__private-info">Only me</span> <span className="annotation-share-info__private-info">Only me</span>
</span> </span>
)} )}
......
...@@ -50,19 +50,16 @@ describe('AnnotationShareInfo', () => { ...@@ -50,19 +50,16 @@ describe('AnnotationShareInfo', () => {
it('should show a link to the group for extant, first-party groups', () => { it('should show a link to the group for extant, first-party groups', () => {
const wrapper = createAnnotationShareInfo(); const wrapper = createAnnotationShareInfo();
const groupLink = wrapper.find('.annotation-share-info__group'); const groupLink = wrapper.find('a');
const groupName = wrapper.find('.annotation-share-info__group-info');
assert.equal(groupLink.prop('href'), fakeGroup.links.html); assert.equal(groupLink.prop('href'), fakeGroup.links.html);
assert.equal(groupName.text(), fakeGroup.name); assert.equal(groupLink.text(), fakeGroup.name);
}); });
it('should display a group icon for private and restricted groups', () => { it('should display a group icon for private and restricted groups', () => {
const wrapper = createAnnotationShareInfo(); const wrapper = createAnnotationShareInfo();
const groupIcon = wrapper.find( const groupIcon = wrapper.find('SvgIcon');
'.annotation-share-info__group .annotation-share-info__icon'
);
assert.equal(groupIcon.prop('name'), 'groups'); assert.equal(groupIcon.prop('name'), 'groups');
}); });
...@@ -71,9 +68,7 @@ describe('AnnotationShareInfo', () => { ...@@ -71,9 +68,7 @@ describe('AnnotationShareInfo', () => {
fakeGroup.type = 'open'; fakeGroup.type = 'open';
const wrapper = createAnnotationShareInfo(); const wrapper = createAnnotationShareInfo();
const groupIcon = wrapper.find( const groupIcon = wrapper.find('SvgIcon');
'.annotation-share-info__group .annotation-share-info__icon'
);
assert.equal(groupIcon.prop('name'), 'public'); assert.equal(groupIcon.prop('name'), 'public');
}); });
......
...@@ -111,12 +111,20 @@ ...@@ -111,12 +111,20 @@
/** /**
* An icon-only button that sits to the right of a text-input field * An icon-only button that sits to the right of a text-input field
* (e.g. "copy to clipboard" button in share panels) * (e.g. "copy to clipboard" button in share panels)
*
* @param {boolean} [$compact] - Tighten padding for small spaces
*/ */
@mixin button--input { @mixin button--input($compact: false) {
@include button; @include button;
@include utils.border; @include utils.border;
@if $compact {
padding: var.$layout-space--xxsmall var.$layout-space--xsmall;
} @else {
padding: var.$layout-space--xsmall var.$layout-space--small;
}
color: var.$grey-mid; color: var.$grey-mid;
padding: 0.5em 0.75em;
background-color: var.$grey-1; background-color: var.$grey-1;
border-radius: 0; // Turn off border-radius to align with <input> edges border-radius: 0; // Turn off border-radius to align with <input> edges
......
...@@ -31,14 +31,26 @@ ...@@ -31,14 +31,26 @@
} }
} }
@mixin form-input { /**
@include utils.font--medium; * A text input field.
*
* @param {boolean} [$compact] - Style for a compact space, with tighter padding
*/
@mixin form-input($compact: false) {
@if $compact {
@include utils.font--small;
padding: var.$layout-space--xsmall;
} @else {
@include utils.font--medium;
padding: var.$layout-space--xsmall var.$layout-space--small;
}
@include utils.border; @include utils.border;
border-radius: var.$border-radius; border-radius: var.$border-radius;
padding: 0.5em 0.75em;
font-weight: normal;
color: var.$color-text-light; color: var.$color-text-light;
background-color: var.$grey-0; background-color: var.$grey-0;
// Tighten up spacing around text in input
line-height: 1;
&:focus { &:focus {
@include form-input-focus; @include form-input-focus;
...@@ -49,12 +61,18 @@ ...@@ -49,12 +61,18 @@
} }
} }
@mixin form-input--with-button { /**
@include form-input; * A text input that has a button to its right; no border radius
*
* @param {boolean} [$compact] - For use in compact areas, with tighter padding
*/
@mixin form-input--with-button($compact: false) {
@include form-input($compact);
width: 100%; width: 100%;
border-radius: 0; border-radius: 0;
} }
// TODO: Deprecate
@mixin primary-action-btn { @mixin primary-action-btn {
@include focus.outline-on-keyboard-focus; @include focus.outline-on-keyboard-focus;
@include utils.font--medium; @include utils.font--medium;
......
...@@ -131,7 +131,11 @@ ...@@ -131,7 +131,11 @@
*/ */
@mixin panel--compact { @mixin panel--compact {
@include panel; @include panel;
padding: 0.75em; width: 20em;
// Keep panel constrained within annotation card boundaries and not cut off
// on left side when sidebar is extremely narrow
max-width: 85vw;
padding: var.$layout-space--small;
&__header { &__header {
padding: 0; padding: 0;
......
...@@ -6,42 +6,52 @@ ...@@ -6,42 +6,52 @@
@use "../../variables" as var; @use "../../variables" as var;
.annotation-share-control { .annotation-share-control {
// Allow pointer arrow to be positioned absolutely relative to this container
position: relative; position: relative;
} }
// A compact panel that appears/disappears by tapping the "share" icon on a
// single annotation.
.annotation-share-panel { .annotation-share-panel {
@include molecules.panel--compact; @include molecules.panel--compact;
// Position panel to align with share-annotation icon and alignment arrow
position: absolute; position: absolute;
right: 5px; right: 5px;
bottom: 32px; bottom: 32px;
width: 275px;
cursor: default;
&__icon-button { @media (pointer: coarse) {
@include buttons.button--input; // Adjust arrow/panel positioning to account for larger icon target
padding: var.$layout-space--xxsmall var.$layout-space--xsmall; right: 13px;
bottom: 40px;
} }
// Override the pointer cursor that applies to the entire annotation card
cursor: default;
&__form-input { &__form-input {
@include forms.form-input--with-button; @include forms.form-input--with-button($compact: true);
@include utils.font--small; }
padding: 0.5em;
&__icon-button {
@include buttons.button--input($compact: true);
} }
&__permissions { &__permissions {
@include utils.font--small; @include utils.font--small;
margin: var.$layout-space--xsmall 0; padding: var.$layout-space--xsmall 0;
} }
// Position the pointer icon absolutely and flip it to make it point at the
// share icon. Fill it with background color and give it the same color as
// the border to make it look like part of the panel frame.
&__arrow { &__arrow {
margin: 0;
color: var.$grey-3;
fill: white;
transform: rotateX(180deg);
position: absolute; position: absolute;
z-index: 100; z-index: 100;
right: 0px; right: 0px;
bottom: -8px; bottom: -8px;
color: var.$color-border;
fill: var.$color-background;
transform: rotateX(180deg);
} }
.share-links__icon { .share-links__icon {
......
...@@ -3,12 +3,6 @@ ...@@ -3,12 +3,6 @@
@use "../../mixins/utils"; @use "../../mixins/utils";
.annotation-share-info { .annotation-share-info {
&__group,
&__private {
@include layout.row($align: baseline);
color: var.$color-text-light;
}
&__icon { &__icon {
@include utils.icon--xsmall; @include utils.icon--xsmall;
// This margin is currently needed because the icon is within an `a` element // This margin is currently needed because the icon is within an `a` element
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment