[terra-time-input] - Mode changing issue on JAWS and SR fixes #2017

PK106552 · 2024-02-05T10:41:49Z

Summary

What was changed:

Changed Up/Down key functionality same as +/- functionality to make uniform.
Changed input type to number instead of text.
Fixed label reading twice issue in time input.

Why it was changed:

Up/Down key functionality was not same as +/- functionality.
Input type was text.
Label value was reading twice in JAWS.

Testing

This change was tested using:

Reviews

In addition to engineering reviews, this PR needs:

UX review
Accessibility review
Functional review

Additional Details

This PR resolves:

UXPLATFORM-6328

Thank you for contributing to Terra.
@cerner/terra

Before fix :

After Fix :

saket2403 · 2024-02-05T11:15:51Z

packages/terra-time-input/src/TimeUtil.js

@@ -436,6 +436,8 @@ TimeUtil.DoubleZeroDigit = '00';

 TimeUtil.initialValue = ['0', '00'];

+TimeUtil.letterKeycode = [38, 40, 65, 66, 67, 68, 69, 70, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90];


What is this array for?

Safari browser allows letters when we have type as number, to prevent this we have to check all the letters keyCode

Can you use regex like this.

let inputValue = e.target.value; let numericValue = inputValue.replace(/[^0-9]/g, ''); e.target.value = numericValue;

handleKeyDown event is triggering when we press any letter, so i have added regex check in keyDown event and removed keyCode array value from TimeUtil file

saket2403 · 2024-02-05T11:17:21Z

Left/Right Arrow key not moving the focus between the input fields. Please compare with previous functionality.

PK106552 · 2024-02-05T11:21:18Z

Left/Right Arrow key not moving the focus between the input fields. Please compare with previous functionality.

The input element's type ('number') does not support setSelectionRange, that's why left and right arrow key is not shifting focus, i'm investigating it.

rbsree · 2024-02-07T07:22:59Z

+1 for Accessibility Review.
The issues mentioned in the PR are fixed now.

The input field for the Terra Time Input is implemented as type="number" field.
The value of the hour or minute field can be changed using the +/- or UP/DOWN arrow key as expected.
Hotkey implemented for the Time Input field "N" is also supported as expected, setting the current time as the input.

Issue:

When users are provided an input in the time field, the user is not able to navigate between the hour and minute fields using the right/left arrow key. When the field is empty the arrow key navigation is working as expected.
The arrow navigation is not a default behavior for , in this case, it has been implemented through the property "setSelectionRange" which works when the input field is empty, but the same is not supported when the "Terra Time Input" field has input defined.
This is not a major blocker for users, as user can still navigate through the input field using TAB/SHIFT+TAB key.

saket2403 · 2024-02-07T10:45:24Z

packages/terra-time-input/tests/wdio/time-input-twelve-hour-spec.js

-  describe('pressing -', () => {
-    it('should roll over to 11 AM if hour and minute is at 12:00 PM', () => {
-      browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/filled-evening');
-      browser.refresh();
-      Terra.hideInputCaret('#timeInput input[name="terra-time-minute-time-input"]');
-
-      $('#timeInput input[name="terra-time-hour-time-input"]').click();
-      browser.keys('12');
-      browser.keys('00');
-      $('#timeInput input[name="terra-time-minute-time-input"]').click();
-      browser.keys('-');
-      expect($('#timeInput input[name="terra-time-hour-time-input"]')).toHaveValue('11');
-      expect($('#timeInput input[name="terra-time-minute-time-input"]')).toHaveValue('59');
-      validateElement('- causes rollover to morning');
-    });
-
-    it('should roll over to 11 PM if hour and minute is at 12:00 AM', () => {
-      browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/filled-morning');
-      browser.refresh();
-      Terra.hideInputCaret('#timeInput input[name="terra-time-minute-time-input"]');
-
-      $('#timeInput input[name="terra-time-hour-time-input"]').click();
-      browser.keys('12');
-      browser.keys('00');
-      $('#timeInput input[name="terra-time-minute-time-input"]').click();
-      browser.keys('-');
-      expect($('#timeInput input[name="terra-time-hour-time-input"]')).toHaveValue('11');
-      expect($('#timeInput input[name="terra-time-minute-time-input"]')).toHaveValue('59');
-      validateElement('- causes rollover to evening');
-    });
-  });
-
-  describe('pressing +', () => {
-    it('should roll over to 12 AM if hour and minute is at 11:59 PM', () => {
-      browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/filled-evening');
-      browser.refresh();
-      Terra.hideInputCaret('#timeInput input[name="terra-time-minute-time-input"]');
-
-      $('#timeInput input[name="terra-time-hour-time-input"]').click();
-      browser.keys('11');
-      browser.keys('59');
-      $('#timeInput input[name="terra-time-minute-time-input"]').click();
-      browser.keys('+');
-      expect($('#timeInput input[name="terra-time-hour-time-input"]')).toHaveValue('12');
-      expect($('#timeInput input[name="terra-time-minute-time-input"]')).toHaveValue('00');
-      validateElement('+ causes rollover to morning');
-    });
-
-    it('should roll over to 12 PM if hour and minute is at 11:59 AM', () => {
-      browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/filled-morning');
-      browser.refresh();
-      Terra.hideInputCaret('#timeInput input[name="terra-time-minute-time-input"]');
-
-      $('#timeInput input[name="terra-time-hour-time-input"]').click();
-      browser.keys('11');
-      browser.keys('59');
-      $('#timeInput input[name="terra-time-minute-time-input"]').click();
-      browser.keys('+');
-      expect($('#timeInput input[name="terra-time-hour-time-input"]')).toHaveValue('12');
-      expect($('#timeInput input[name="terra-time-minute-time-input"]')).toHaveValue('00');
-      validateElement('+ causes rollover to evening');
-    });
-  });
-


Why are these tests removed? Instead the click events before pressing +/- should be updated so that they match the expected output.

saket2403 · 2024-02-07T13:37:17Z

packages/terra-time-input/tests/wdio/time-input-twelve-hour-spec.js

-  it('displays twelve hour meridiem - Up Arrow on hour does not change meridiem', () => {
-    browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/default');
-    browser.refresh();
-    Terra.hideInputCaret('#timeInput input[name="terra-time-hour-time-input"]');
-
-    $('#timeInput input[name="terra-time-hour-time-input"]').click();
-    browser.keys(['ArrowUp']);
-    validateRoot('up arrow does not change meridiem');
-  });
-
-  it('displays twelve hour - Changes time to 01 when up is pressed on hour of 12', () => {
-    browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/default');
-    browser.refresh();
-    Terra.hideInputCaret('#timeInput input[name="terra-time-hour-time-input"]');
-
-    $('#timeInput input[name="terra-time-hour-time-input"]').click();
-    browser.keys('12');
-    $('#timeInput input[name="terra-time-hour-time-input"]').click();
-    browser.keys(['ArrowUp']);
-    validateRoot('up arrow changes time to 01');
-  });
-
-  it('displays twelve hour - Changes time to 12 when down is pressed on hour of 01', () => {
-    browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/default');
-    browser.refresh();
-    Terra.hideInputCaret('#timeInput input[name="terra-time-hour-time-input"]');
-
-    $('#timeInput input[name="terra-time-hour-time-input"]').click();
-    browser.keys('01');
-    $('#timeInput input[name="terra-time-hour-time-input"]').click();
-    browser.keys(['ArrowDown']);
-
-    validateRoot('down arrow changes time to 12');
-  });
-
-  it('displays twelve hour meridiem - Switched when up is press on hour of 11', () => {
-    browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/default');
-    browser.refresh();
-    Terra.hideInputCaret('#timeInput input[name="terra-time-hour-time-input"]');
-
-    $('#timeInput input[name="terra-time-hour-time-input"]').click();
-    browser.keys('11');
-    $('#timeInput input[name="terra-time-hour-time-input"]').click();
-    browser.keys(['ArrowUp']);
-
-    validateRoot('switch meridiem - up arrow');
-  });
-
-  it('displays twelve hour meridiem - Switched when down is press on hour of 12', () => {
-    browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/default');
-    browser.refresh();
-    Terra.hideInputCaret('#timeInput input[name="terra-time-hour-time-input"]');
-
-    $('#timeInput input[name="terra-time-hour-time-input"]').click();
-    browser.keys('12');
-    $('#timeInput input[name="terra-time-hour-time-input"]').click();
-    browser.keys(['ArrowDown']);
-    validateRoot('switch meridiem - down arrow');
-  });
-


Please update these tests so that they test the newly added functionality of the Up/Down arrow keys instead of removing them.

saket2403 · 2024-02-07T13:38:10Z

packages/terra-time-input/tests/wdio/time-input-twelve-hour-spec.js

@@ -126,29 +66,28 @@ Terra.describeViewports('Time Input Twelve Hour', ['medium'], () => {

  describe('pressing -', () => {
    it('should roll over to 11 AM if hour and minute is at 12:00 PM', () => {
-      browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/filled-evening');
+      browser.url('/raw/tests/cerner-terra-framework-docs/time-input/twelve-hour/default');


Why are these URLs changed what was the issue with the existing url? I can see similar changes in other tests also

Typed input is not rendering If we use filled input field, that's why i have used default empty field test example

So that means the existing functionality is breaking. In previous implementation you could type without clearing the field. Please try to fix.

I have verified it, previous implementation is working fine, we can able to edit filled time when we navigate using tab key, but when we click on field using mouse that time only we could not able to edit the filled time.

So please look into it if that can be fixed

sugan2416 · 2024-02-09T09:48:28Z

packages/terra-time-input/src/TimeInput.jsx

+      event.currentTarget.value = ''; // eslint-disable-line no-param-reassign
+    }
+
+    if (!event.key.match(/^[0-9]/g) && !(event.keyCode === KeyCode.KEY_BACK_SPACE || event.keyCode === KeyCode.KEY_DELETE || event.keyCode === KeyCode.KEY_TAB || event.keyCode === KeyCode.KEY_RIGHT || event.keyCode === KeyCode.KEY_LEFT)) {


should we not include Up/Down arrows here ?

No need, because automatically Up/Down key will be increase/decrease value when we use type as number in input field

sugan2416 · 2024-02-09T10:07:13Z

packages/terra-time-input/src/TimeInput.module.scss

@@ -311,4 +311,10 @@
      }
    }
  }
+  // stylelint-disable property-no-vendor-prefix
+  input::-webkit-outer-spin-button,


why was this added? Was it to suppress warning?

This style was included to hide arrow icons in input field when we use type as number, it was to suppress property-no-vendor-prefix warning

saket2403 · 2024-02-09T13:14:59Z

packages/terra-date-time-picker/tests/wdio/date-time-picker-spec.js

-          const secondsValue = Number($('input[name="terra-time-second-input"]').getValue());
-          const secondsTimeValue = today.seconds();
-          const secondsInRange = (secondsTimeValue === secondsValue + 1 || secondsTimeValue === secondsValue);
-          expect(secondsInRange).toBe(true);


Why has this been removed?

saket2403 · 2024-02-09T13:15:10Z

packages/terra-date-time-picker/tests/wdio/date-time-picker-spec.js

-          const secondsValue = Number($('input[name="terra-time-second-input"]').getValue());
-          const secondsTimeValue = today.seconds();
-          const secondsInRange = (secondsTimeValue === secondsValue + 1 || secondsTimeValue === secondsValue);
-          expect(secondsInRange).toBe(true);


Why has this been removed?

saket2403 · 2024-02-09T13:17:54Z

packages/terra-date-time-picker/tests/wdio/date-time-picker-spec.js

+        it('should set date to today and now plus 1 seconds in the second input', () => {
          browser.url('/raw/tests/cerner-terra-framework-docs/date-time-picker/date-time-picker-with-seconds');
          browser.refresh();
          $('input[name="terra-time-second-input"]').click();
          browser.keys('+');

-          const today = moment.utc().add(1, 'minutes');
+          const today = moment.utc().add(1, 'seconds');
          expect($('input[name="terra-date-year-input"]')).toHaveValue(today.year().toString());
          expect($('input[name="terra-date-month-input"]')).toHaveValue((`0${(today.month() + 1)}`).slice(-2));
          expect($('input[name="terra-date-day-input"]')).toHaveValue((`0${today.date()}`).slice(-2));
          expect($('input[name="terra-time-hour-input"]')).toHaveValue((`0${today.hour()}`).slice(-2));
          expect($('input[name="terra-time-minute-input"]')).toHaveValue((`0${today.minute()}`).slice(-2));
-          const secondsValue = Number($('input[name="terra-time-second-input"]').getValue());
-          const secondsTimeValue = today.seconds();
-          const secondsInRange = (secondsTimeValue === secondsValue + 1 || secondsTimeValue === secondsValue);
-          expect(secondsInRange).toBe(true);


This test says that you are testing +1 sec to the seconds but I cannot see the validation for seconds field here.

saket2403 · 2024-02-09T13:24:45Z

packages/terra-date-time-picker/tests/wdio/date-time-picker-spec.js

-          const secondsValue = Number($('input[name="terra-time-second-input"]').getValue());
-          const secondsTimeValue = today.seconds();
-          const secondsInRange = (secondsTimeValue === secondsValue + 1 || secondsTimeValue === secondsValue);
-          expect(secondsInRange).toBe(true);


This test says that you are testing +1 sec to the seconds but you have removed seconds field validation

Time input fixes

c65eafb

github-actions bot temporarily deployed to preview-pr-2017 February 5, 2024 10:42 Destroyed

PK106552 added the 📦 terra-time-input label Feb 5, 2024

PK106552 self-assigned this Feb 5, 2024

PK106552 added the Accessibility Review Required label Feb 5, 2024

PK106552 requested review from sugan2416 and saket2403 February 5, 2024 10:42

saket2403 reviewed Feb 5, 2024

View reviewed changes

Updated WDIO screeshots

b0958bd

github-actions bot temporarily deployed to preview-pr-2017 February 5, 2024 16:54 Destroyed

Fixed lint erroe

f7dcd76

github-actions bot temporarily deployed to preview-pr-2017 February 5, 2024 17:12 Destroyed

Updated WDIO screenshots

e073d2e

github-actions bot temporarily deployed to preview-pr-2017 February 5, 2024 17:52 Destroyed

Fixed build fail

bc5bcef

github-actions bot temporarily deployed to preview-pr-2017 February 7, 2024 06:47 Destroyed

PK106552 added ⭐ Accessibility Reviewed and removed Accessibility Review Required labels Feb 7, 2024

PK106552 requested a review from saket2403 February 7, 2024 07:28

saket2403 reviewed Feb 7, 2024

View reviewed changes

Updated WDIO

4d8461d

github-actions bot temporarily deployed to preview-pr-2017 February 7, 2024 12:15 Destroyed

PK106552 requested a review from saket2403 February 7, 2024 12:16

saket2403 reviewed Feb 7, 2024

View reviewed changes

Updated WDIO

d101e79

github-actions bot temporarily deployed to preview-pr-2017 February 7, 2024 16:57 Destroyed

github-actions bot temporarily deployed to preview-pr-2017 February 8, 2024 08:00 Destroyed

updated WDIO

c2bcf0c

github-actions bot temporarily deployed to preview-pr-2017 February 8, 2024 08:45 Destroyed

sugan2416 reviewed Feb 9, 2024

View reviewed changes

PK106552 requested a review from sugan2416 February 9, 2024 10:51

sugan2416 approved these changes Feb 9, 2024

View reviewed changes

sugan2416 self-requested a review February 9, 2024 11:23

sugan2416 approved these changes Feb 9, 2024

View reviewed changes

github-actions bot temporarily deployed to preview-pr-2017 February 9, 2024 11:54 Destroyed

Merge branch 'main' into timeInput_fixes

885cc4d

PK106552 force-pushed the timeInput_fixes branch from d8c1171 to 885cc4d Compare February 9, 2024 12:28

github-actions bot temporarily deployed to preview-pr-2017 February 9, 2024 12:28 Destroyed

Fixed wdio

c40ecea

github-actions bot temporarily deployed to preview-pr-2017 February 9, 2024 12:46 Destroyed

saket2403 reviewed Feb 9, 2024

View reviewed changes

Updated wdio

fb6f7ab

github-actions bot temporarily deployed to preview-pr-2017 February 9, 2024 15:17 Destroyed

Added second validation

83bc3ca

github-actions bot temporarily deployed to preview-pr-2017 February 9, 2024 15:24 Destroyed

Fixed lint

e10da89

github-actions bot temporarily deployed to preview-pr-2017 February 9, 2024 15:46 Destroyed

reverted removed test cases

a7adf38

github-actions bot temporarily deployed to preview-pr-2017 February 9, 2024 16:10 Destroyed

saket2403 approved these changes Feb 12, 2024

View reviewed changes

saket2403 merged commit 50c3420 into main Feb 12, 2024
22 checks passed

saket2403 deleted the timeInput_fixes branch February 12, 2024 09:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[terra-time-input] - Mode changing issue on JAWS and SR fixes #2017

[terra-time-input] - Mode changing issue on JAWS and SR fixes #2017

PK106552 commented Feb 5, 2024

saket2403 Feb 5, 2024

PK106552 Feb 5, 2024

saket2403 Feb 5, 2024

PK106552 Feb 5, 2024

saket2403 commented Feb 5, 2024

PK106552 commented Feb 5, 2024

rbsree commented Feb 7, 2024 •

edited

Loading

saket2403 Feb 7, 2024

PK106552 Feb 7, 2024

saket2403 Feb 7, 2024

PK106552 Feb 7, 2024

saket2403 Feb 7, 2024 •

edited

Loading

PK106552 Feb 7, 2024

saket2403 Feb 7, 2024

PK106552 Feb 7, 2024

saket2403 Feb 7, 2024

PK106552 Feb 7, 2024

sugan2416 Feb 9, 2024 •

edited

Loading

PK106552 Feb 9, 2024

sugan2416 Feb 9, 2024

PK106552 Feb 9, 2024

saket2403 Feb 9, 2024

PK106552 Feb 9, 2024

saket2403 Feb 9, 2024

PK106552 Feb 9, 2024

saket2403 Feb 9, 2024

PK106552 Feb 9, 2024

saket2403 Feb 9, 2024

PK106552 Feb 9, 2024

		@@ -436,6 +436,8 @@ TimeUtil.DoubleZeroDigit = '00';

		TimeUtil.initialValue = ['0', '00'];

		TimeUtil.letterKeycode = [38, 40, 65, 66, 67, 68, 69, 70, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90];

[terra-time-input] - Mode changing issue on JAWS and SR fixes #2017

[terra-time-input] - Mode changing issue on JAWS and SR fixes #2017

Conversation

PK106552 commented Feb 5, 2024

Summary

Testing

Reviews

Additional Details

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

saket2403 commented Feb 5, 2024

PK106552 commented Feb 5, 2024

rbsree commented Feb 7, 2024 • 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

saket2403 Feb 7, 2024 • 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

sugan2416 Feb 9, 2024 • 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

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

rbsree commented Feb 7, 2024 •

edited

Loading

saket2403 Feb 7, 2024 •

edited

Loading

sugan2416 Feb 9, 2024 •

edited

Loading