Skip to content

Refactored Calendar component to remove dependency of primereact. (#2592 - #2592

Merged
Keith-CY merged 14 commits into
nervosnetwork:developfrom
mortoys:remove-primereact
Mar 25, 2023
Merged

Refactored Calendar component to remove dependency of primereact. (#2592#2592
Keith-CY merged 14 commits into
nervosnetwork:developfrom
mortoys:remove-primereact

Conversation

@mortoys

@mortoys mortoys commented Feb 23, 2023

Copy link
Copy Markdown
Contributor

Hi everyone,

I've reimplemented the new Calendar component as described below issue, and removed the dependency on primereact.

It generally follows the previous interactive logic, keeps the current system's colour and style, and adds a check for the available range of dates.

Any suggestions for improvement would be appreciated.

Magickbase/neuron-public-issues#66

Kapture 2023-02-23 at 14 56 09

Comment thread packages/neuron-ui/src/widgets/Calendar/calendar.module.scss Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/utils.ts Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/utils.ts Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/utils.ts Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/utils.ts Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/calendar.module.scss Outdated
Comment thread packages/neuron-ui/src/widgets/DatetimePicker/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/DatetimePicker/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/utils.ts Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/utils.ts Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Outdated
@Keith-CY

Copy link
Copy Markdown
Contributor

CI failed

Comment thread packages/neuron-ui/src/widgets/DatetimePicker/index.tsx Outdated
@Keith-CY

Copy link
Copy Markdown
Contributor

@Cedar67 please have a review on the functionality

@Keith-CY

Keith-CY commented Mar 3, 2023

Copy link
Copy Markdown
Contributor

How is it going @Cedar67

@Cedar67

Cedar67 commented Mar 3, 2023

Copy link
Copy Markdown
Contributor

How is it going @Cedar67

Two other high-priority issues are currently under validation. This PR will be arranged to complete the verification next Monday.

@Cedar67

Cedar67 commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

@mortoys The display style for the current day and the selected date should remain the same as before.

image

@Cedar67

Cedar67 commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

Need to add a period at the end of each month abbreviation.

image

@Cedar67

Cedar67 commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

One exception: When selecting another month by the arrow symbol, it automatically jumps back to the current month.

Screen.Recording.2023-03-06.at.2.31.40.PM.mov

@mortoys

mortoys commented Mar 6, 2023

Copy link
Copy Markdown
Contributor Author

Need to add a period at the end of each month abbreviation.

image

According to the current i18n method, this cannot be controlled

@Cedar67

Cedar67 commented Mar 7, 2023

Copy link
Copy Markdown
Contributor

Need to add a period at the end of each month abbreviation.
image

According to the current i18n method, this cannot be controlled

@mortoys Received. Thanks for your feedback. This product requirement change confirmation has been submitted to PM. Wait for his feedback.

@Cedar67

Cedar67 commented Mar 7, 2023

Copy link
Copy Markdown
Contributor

One exception: When selecting another month by the arrow symbol, it automatically jumps back to the current month.

Verified.

@Cedar67

Cedar67 commented Mar 7, 2023

Copy link
Copy Markdown
Contributor

@mortoys The display style for the current day and the selected date should remain the same as before.

image

Verified.

@Keith-CY

Keith-CY commented Mar 7, 2023

Copy link
Copy Markdown
Contributor

Need to add a period at the end of each month abbreviation.
image

According to the current i18n method, this cannot be controlled

It can be done by updating here https://github.com/nervosnetwork/neuron/pull/2592/files#diff-227821703ffb93fc0998891374bc9394fb37fb7bfb38486fd284b9b0c5aaef69R98-R101

const getLocalMonthNames = (lang: string) => {
  const formater = new Intl.DateTimeFormat(lang, { month: 'short' })
  return Array.from({ length: 12 }, (_, i) => `${formater.format(new Date(Date.UTC(2023, i, 1)))}${lang.startsWith('en') ? '.' : ''}`)
}

@mortoys

mortoys commented Mar 7, 2023

Copy link
Copy Markdown
Contributor Author

I found some calendar accessibility examples here:https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/examples/datepicker-dialog/
https://react-spectrum.adobe.com/react-aria/useCalendar.html

and a great wai-aria lesson: https://www.udacity.com/course/web-accessibility--ud891
I plan to upgrade the component more accessibility

@mortoys

mortoys commented Mar 13, 2023

Copy link
Copy Markdown
Contributor Author

Add focus style
Add wai-aria labels and related i18n language
Add focus fully controlled with the keyboard:

  1. focus can be controlled by Tab and Shift + Tab to move forward and backward
  2. Space and Enter to select
  3. Up Arrow Down Arrow Right Arrow Left Arrow control the direction of movement
  4. Home first day of the current month. End last day of the current month.
  5. Page Up previous month Page Down next month

It can be tested use Mac Voiceover(press Command-F5), and feel whether the blind can be used properly
Also you can check accessibility tree in Chrome DevTools.

It's still a lot to do, e.g.

  • The modal must trap the focus

Kapture 2023-03-13 at 17 54 25

Comment thread packages/neuron-ui/src/widgets/Calendar/index.tsx Fixed
@Cedar67

Cedar67 commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Code scanning and Unit test results indicate exceptions that need to be confirmed.

Comment thread packages/neuron-ui/src/widgets/Calendar/focusControl.tsx
Comment thread packages/neuron-ui/src/widgets/Calendar/focusControl.tsx Outdated
@Keith-CY

Copy link
Copy Markdown
Contributor

@Cedar67

Cedar67 commented Mar 18, 2023

Copy link
Copy Markdown
Contributor

Cannot be saved after the year is changed.

Screen.Recording.2023-03-19.at.7.20.41.AM.mov

@Cedar67

Cedar67 commented Mar 18, 2023

Copy link
Copy Markdown
Contributor

Need to add a period at the end of each month abbreviation.

image

This issue has been Solved and Verified.

@Keith-CY

Copy link
Copy Markdown
Contributor

Need to add a period at the end of each month abbreviation.
image

This issue has been Solved and Verified.

Please mark this PR approved if tests are all green

@Cedar67

Cedar67 commented Mar 18, 2023

Copy link
Copy Markdown
Contributor

Please mark this PR approved if tests are all green

Found a new issue to be confirmed by the developer.

#2592 (comment)

@mortoys

mortoys commented Mar 19, 2023

Copy link
Copy Markdown
Contributor Author

@Cedar67
Changes will only be submitted after the date is selected in the monthly calendar
this interaction is same as the past

@Keith-CY Keith-CY changed the title Refactored Calendar component to remove dependency of primereact. Refactored Calendar component to remove dependency of primereact. (#2592 Mar 25, 2023
@Keith-CY
Keith-CY merged commit 9e559cb into nervosnetwork:develop Mar 25, 2023
@Keith-CY

Copy link
Copy Markdown
Contributor

@Cedar67 Changes will only be submitted after the date is selected in the monthly calendar this interaction is same as the past

Appreciate your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants