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

feat: add date-picker today actions #1058

Merged
merged 10 commits into from Jun 11, 2020
Merged

Conversation

prsdthkr
Copy link
Member

@prsdthkr prsdthkr commented May 27, 2020

Description

  • Add 2 optional features to the date-picker:
    • A 'today selection' button in datepicker footer that selects today's date, closes the datepicker, and updates the input field.
    • A 'today navigation' button in datepicker header that focuses on today's date, selects it but does not close the datepicker. Useful in mobile use cases where datepicker is closed after 'OK' button is clicked.
  • Fix compact and cosy date-picker styles

Screenshots

image

image

Please check whether the PR fulfills the following requirements

@prsdthkr prsdthkr self-assigned this May 27, 2020
@netlify
Copy link

netlify bot commented May 27, 2020

Deploy preview for fundamental-styles ready!

Built with commit 7b6171c

https://deploy-preview-1058--fundamental-styles.netlify.app

@prsdthkr prsdthkr marked this pull request as ready for review May 27, 2020 18:46
@prsdthkr prsdthkr requested a review from a team May 27, 2020 18:49
@prsdthkr prsdthkr added Doc website documentation website Pattern labels May 27, 2020
@prsdthkr prsdthkr requested a review from a team May 27, 2020 19:52
@InnaAtanasova
Copy link
Contributor

hey @prsdthkr how would this button look like in mobile view?
In mobile view the calendar has the OK button in the footer.
Screen Shot 2020-05-28 at 1 25 40 PM

Does it mean the Today button will be placed next to it? And if yes, is it a good UX?

@prsdthkr
Copy link
Member Author

Hi @InnaAtanasova, good catch. After looking at the Fiori 3 specs I understand that in mobile view the datepicker should be rendered WITHIN a full-screen dialog component. This means a today button should render like this mock:
image

I'm not sure if this is a good user experience. We may need some guidance from SAP UX. What do you think?

@bcullman
Copy link
Contributor

bcullman commented May 28, 2020

in the desktop view, selecting a date (or selecting today) is "closing the calendar" event.

if in the moible view if selecting a date does not close the calendar, then neither should the today button. if that is the case, the today button is a navigation element, and should be in the header. This is what i gathered from my conversation with SAP UX.

My question is "why" does selecting a date not dismiss the calendar on mobile?

@salarenko
Copy link
Contributor

@InnaAtanasova @prsdthkr I think we should ping this to the designers and also include mobile example in pattern section.

@prsdthkr Could you by the way fix calendar navigation buttons?
Normal size partially uses compact buttons and compact size partially uses normal buttons 😋
Removing/adding fd-button--compact will do the thing.

@stefanoScalzo
Copy link
Contributor

PR title should not be docs because it is changing the markup

@JKMarkowski
Copy link
Contributor

JKMarkowski commented Jun 2, 2020

@stefanoScalzo I'm not sure if docs is not good there. We do not provide any breaking change, so markup doesn't have to be changed.

Copy link
Contributor

@jbadan jbadan left a comment

Choose a reason for hiding this comment

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

I would mark it as breaking for removing/adding fd-button--compact class, the footer additions aren't breaking unless they're required.

@prsdthkr prsdthkr changed the title docs: add date-picker today button as footer action feat: add date-picker today button as footer action Jun 3, 2020
@prsdthkr
Copy link
Member Author

prsdthkr commented Jun 3, 2020

Based on inputs from SAP UX and @bcullman, the today button will act as a navigational element in mobile view. Hence it will appear in the "upper right most corner" according to inputs from SAP UX. I have mocked this below. Would like to know what the designers think. Ideally we would be providing two today buttons (header and footer) and users can enable these based on desktop or mobile use case.

image

image

@bcullman
Copy link
Contributor

bcullman commented Jun 4, 2020

per this PR description, it shows the “today” button as part of the default datepicker (before/after).

Instead, the today button should be implemented as a feature, that is OFF by default.

additionally, consider creating 2 today buttons

  • one called "today-select" which is the original feature you worked on - a today button in the footer that selects today's date and closes the date picker. it is off by default
  • once called "today-nav" which can be used in the mobile scenario - a today button in the header that navigates to, but does not select, today's date. it is off by default

iteration on the style of a "today-nav" can be done in a follow up PR, as long as the semantic HTML is in place, someone else can refine that style at a later when it's required.

@stefanoScalzo
Copy link
Contributor

You are using aria controls but there is no element with that id
aria-controls="D8NDL125"

@stefanoScalzo
Copy link
Contributor

Can you please include the mobile version in the examples please

@prsdthkr
Copy link
Member Author

prsdthkr commented Jun 8, 2020

@stefanoScalzo I've added the DatePicker examples for mobile landscape and portrait mode with today navigation, based on the corresponding examples in Calendar. Should the mobile examples from Calendar be removed? Also now that I think about this, should the today navigation button be part of the Calendar spec?

Copy link
Contributor

@JKMarkowski JKMarkowski left a comment

Choose a reason for hiding this comment

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

LGTM

@prsdthkr prsdthkr changed the title feat: add date-picker today button as footer action feat: add date-picker today actions Jun 11, 2020
@prsdthkr prsdthkr merged commit e4d8a4a into master Jun 11, 2020
@prsdthkr prsdthkr deleted the docs/date-picker-today-button branch June 11, 2020 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Doc website documentation website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants