✨ RSP-631 InputGroup: add Datepicker range variant #53

majornista · 2018-12-12T23:07:09Z

Add Datepicker range variants to InputGroup to support date and datetime range.

Description

Add .spectrum-Datepicker--range and .spectrum-Datepicker--datetimeRange to .spectrum-InputGroup style definitions.

Update docs to support adding .is-focused style to InputGroup on focus, similar to how we do it for .spectrum-Slider.

Related Issues

#54
✨ RSP-631: Datepicker range styles added
Spectrum/spectrum-css/issues/663
JIRA issue RSP-631

Motivation and Context

Calendar supports range selection but Datepicker component does not display the date range.

How Has This Been Tested?

Tested in Spectrum docs and as part of a related React-Spectrum PR.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.

lazd

This is almost good to go, a few changes and we can merge. I'd like to get design guidance on some of the widths/presentation, otherwise it's just implementation details.

src/inputgroup/index.css

lazd · 2018-12-17T17:36:18Z

src/inputgroup/index.css


 :root {
 /* Todo: move to DNA */
 --spectrum-combobox-quiet-fieldbutton-border-radius: 0;
 --spectrum-combobox-field-border-width-right: 0;
 --spectrum-combobox-quiet-fieldbutton-padding-right: 0;
+ --spectrum-datepicker-input-width: calc(var(--spectrum-global-dimension-size-1600) - 2 * var(--spectrum-padding));
+ --spectrum-datepicker-datetime-input-width: calc(var(--spectrum-datepicker-input-width) + var(--spectrum-global-dimension-size-700) - 0.5em);


Perhaps @NateBaldwinDesign can provide guidance on the right width to use here? Also, I assume this will work fine with dates using a period or - separator instead of / (the date won't overlap the em dash)?

docs/inputgroup/datepicker-range-quiet.yml

src/inputgroup/skin.css

docs/inputgroup/datepicker-range.yml

topdoc/resources/js/docs.js

lazd · 2018-12-19T19:19:43Z

src/inputgroup/index.css

+ padding-right: var(--spectrum-padding);
+ }
+ }
+ &:nth-of-type(2) {


@GarthDB how does this make you feel? Do you think we should have selectors for these two bad boys instead of doing nth-of-type?

It has full browser support, so I don't really have any issue with it. https://caniuse.com/#feat=css-sel3

The only other reason we might be concerned is performance and/or code readability/maintainability, but I think it's fine on both of those.

I think the specificity is pretty high here. I'm not 100% against it, but it feels like we could throw a class on the inputs and simplify the CSS... The real question is: do we want markup simplicity (the PR, as-is), or CSS simplicity (throw an extra class on the fields)?

@majornista We talked a bit with @devongovett and think these selectors should be classes added to the markup.

@lazd Would spectrum-Datepicker-field--start and spectrum-Datepicker-field--end work?

lazd · 2018-12-19T19:20:13Z

@majornista almost good to go! Just those icons remaining, and pending some feedback from @GarthDB on the nth-of-type situation.

GarthDB · 2019-01-07T18:00:38Z

@majornista is this a duplicate of #13 ?

majornista · 2019-01-07T18:20:23Z

@majornista is this a duplicate of #13 ?

Yes, this supersedes #13.

GarthDB · 2019-01-07T18:21:20Z

@majornista perfect, I'll close that one in favor of this.

majornista · 2019-02-20T20:52:20Z

@GarthDB, @lazd and @NateBaldwinDesign Where are we with this one?

Update initial margin on hr and em dash.

Use div with class name rather than hr for styling dash.

Improve calculation of dash offset on input.

Fix vertical alignment of em-dash for Medium and Large size variants.

src/inputgroup/index.css

src/inputgroup/skin.css

lazd · 2019-03-15T16:48:46Z

docs/inputgroup/datepicker-range.yml

+ </svg>
+ </button>
+ <!-- To render focus ring around entire input group when one of the inputs has keyboard focus, we need this: -->
+ <div class="spectrum-Datepicker-focusRing" role="presentation"></div>


Mmmm, this can't be done with :before/:after? I don't see anything using those pseudo-elements right now.

@majornista oh, I see -- it's being selected with the sibling selector, so we can't use pseudo-elements. Perhaps, instead, we should be programmatically applying .is-focused to the outer .spectrum-Datepicker and using :focus-within (for futureproofing)

Only with additional javascript to check for .focus-ring className on a descendant element. And, we'd probably need an extra .focus-ring state, to distinguish between mouse a keyboard focus. With the added element, the style can be applied with pure CSS.

Got it, so we would need to know "is a child element keyboard focused?" in JavaScript, and a separate class for keyboard vs mouse focus. @GarthDB thoughts on this? This is the first component that has anything like this.

I prefer pure CSS for accessibility things like keyboard focus. It would be preferable to implement it with the same strategy as all the others, but I'm not seeing an obvious way to do that.
We could add something to the focus-ring polyfill, but that would kind of defeat the point of a standards-based polyfill.
The focus-withing strategy would be nice, it just means we're adding another polyfill requirement to focus. What would you prefer @lazd?

Though I would prefer the polyfill, I don't want to hold up this PR. I agree with your sentiments and suppose I'm fine with the current approach.

Got it, so we would need to know "is a child element keyboard focused?" in JavaScript, and a separate class for keyboard vs mouse focus. @GarthDB thoughts on this? This is the first component that has anything like this.

The Rating component has a similar issue with how the focus outline is applied to the wrapper element. Devon was somewhat resistant to the approach of evaluating for descendant input with .focus-ring. The difference for the Datepicker is that text inputs show blue border for mouse focus and border with box-shadow for keyboard focus.

lazd · 2019-03-18T18:07:18Z

Thanks for working through all of the changes, merging!

majornista changed the title ~~✨ RSP-631 InputGroup: add Datepicker range variant~~ ✨ RSP-631 InputGroup: add Datepicker range variant Dec 12, 2018

lazd force-pushed the master branch from f4dddce to b9a274b Compare December 13, 2018 00:11

lazd requested changes Dec 17, 2018

View reviewed changes

majornista force-pushed the RSP-631 branch from 3162ba6 to 1eb6e42 Compare December 18, 2018 22:38

lazd reviewed Dec 19, 2018

View reviewed changes

majornista force-pushed the RSP-631 branch from 229567e to 72ba73d Compare January 3, 2019 15:49

GarthDB mentioned this pull request Jan 7, 2019

✨ RSP-631: Datepicker range styles added #13

Closed

majornista added 6 commits March 11, 2019 19:19

RSP-631 InputGroup: add Datepicker range variant

e080337

RSP-631 InputGroup: Datepicker range use em dash rather than en dash

65d896d

Update initial margin on hr and em dash.

RSP-631 InputGroup: Datepicker range adjust dash based on text width

1e12507

Use div with class name rather than hr for styling dash.

RSP-631 InputGroup: Datepicker range updates per comments

dab1f78

Improve calculation of dash offset on input.

RSP-631 InputGroup: Datepicker range code comments

aad94e3

RSP-631 InputGroup: Datepicker fix vertical alignment of em-dash

595750b

Fix vertical alignment of em-dash for Medium and Large size variants.

majornista force-pushed the RSP-631 branch from 72ba73d to 595750b Compare March 12, 2019 02:22

lazd reviewed Mar 13, 2019

View reviewed changes

src/inputgroup/index.css Outdated Show resolved Hide resolved

lazd reviewed Mar 13, 2019

View reviewed changes

src/inputgroup/skin.css Outdated Show resolved Hide resolved

RSP-631 Simplify em-dash positioning using flexbox

2e822b9

lazd reviewed Mar 15, 2019

View reviewed changes

majornista and others added 2 commits March 15, 2019 15:32

RSP-631 Use className rather than :nth-of-type pseudo-selector

7366bb3

Fix naming convention for start/end fields

120e9fe

lazd approved these changes Mar 18, 2019

View reviewed changes

lazd merged commit 5660100 into adobe:master Mar 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ RSP-631 InputGroup: add Datepicker range variant #53

✨ RSP-631 InputGroup: add Datepicker range variant #53

majornista commented Dec 12, 2018 •

edited

Loading

lazd left a comment

lazd Dec 17, 2018

lazd Dec 19, 2018

GarthDB Jan 7, 2019

GarthDB Jan 7, 2019

lazd Mar 13, 2019

lazd Mar 15, 2019

majornista Mar 15, 2019 •

edited

Loading

lazd commented Dec 19, 2018

GarthDB commented Jan 7, 2019

majornista commented Jan 7, 2019

GarthDB commented Jan 7, 2019

majornista commented Feb 20, 2019 •

edited

Loading

lazd Mar 15, 2019

lazd Mar 15, 2019

majornista Mar 15, 2019

lazd Mar 15, 2019

GarthDB Mar 15, 2019

lazd Mar 15, 2019

majornista Mar 15, 2019

lazd commented Mar 18, 2019

✨ RSP-631 InputGroup: add Datepicker range variant #53

✨ RSP-631 InputGroup: add Datepicker range variant #53

Conversation

majornista commented Dec 12, 2018 • edited Loading

Description

Related Issues

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

lazd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majornista Mar 15, 2019 • edited Loading

Choose a reason for hiding this comment

lazd commented Dec 19, 2018

GarthDB commented Jan 7, 2019

majornista commented Jan 7, 2019

GarthDB commented Jan 7, 2019

majornista commented Feb 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lazd commented Mar 18, 2019

majornista commented Dec 12, 2018 •

edited

Loading

majornista Mar 15, 2019 •

edited

Loading

majornista commented Feb 20, 2019 •

edited

Loading