-
Notifications
You must be signed in to change notification settings - Fork 18
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(date-picker): add date picker component #550
base: main
Are you sure you want to change the base?
Conversation
🚀 Deploying to staging environment Username: |
Datepicker.prototype.createDialogMarkup = function (titleId) { | ||
return `<div class="moj-datepicker__dialog__header "> | ||
<div class="moj-datepicker__dialog__navbuttons"> | ||
<button class="js-datepicker-prev-year" aria-label="previous year" data-button="button-datepicker-prevyear"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
js-datepicker-prev-year
needs to be moj-
namespaced
aria-label
isn't needed if <span class="govuk-visually-hidden">Previous year</span>
is included below
I don't think data-button="button-datepicker-prevyear"
is used
I wonder if we could/should use secondary GDS buttons as a basis here
// create cell (day) | ||
const cell = document.createElement('td') | ||
const dateButton = document.createElement('button') | ||
dateButton.dataset.form = 'date-select' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what .dataset.form
is doing here or if it's needed.
We might need to add aria-label
to explain what the button does (e.g. "28th January 2024"), but I'm not sure what state of the art is here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some general thoughts:
- The popup doesn't close on clickaway, which could lead to a violation of WCAG 2.2 Focus Not Obscured
- If JavaScript isn't available, it doesn't gracefully degrade as the button is still visible. The button should be added by JavaScript rather than in the template HTML.
- In general, I think the template shouldn't contain form groups, error messages, etc. It should just be
{{ govukInput({attributes: {'data-module': 'moj-date-picker'}}) }}
- In general, I think the template shouldn't contain form groups, error messages, etc. It should just be
- Whilst we haven't back-filled it to existing components, we should make sure this has localisation support for all text on release
Can be initialised in Nunjucks with the `mojDatePicker` macro, or in JavaScript with the `data-module="moj-date-picker"` attribute. Fixes MDS-72
f9c79aa
to
a35d795
Compare
Can be initialised in Nunjucks with the
mojDatePicker
macro, or in JavaScript with thedata-module="moj-date-picker"
attribute.Fixes MDS-72