Numberfield followup #1029

snowystinger · 2020-08-28T19:18:47Z

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

Mostly a save point, but this starts to address style issues Labels the field correctly Adds some stories Moves specs because it's more complicated and easier to read on its own

…t cover active/hover states buttons are now on the right side

Don't allow values less than the min or greater than the max Add many TODO's Use NaN in to be a little more explicit about what's happening, plus it's a number Fix a lot of tests, remove a lot of v2 tests because we're pretty different than v2

adobe-bot · 2020-08-28T19:28:49Z

Build successful! 🎉

Fix labels Add label test

adobe-bot · 2020-08-31T16:59:20Z

Build successful! 🎉

# Conflicts: # packages/@react-aria/numberfield/src/useNumberField.ts # packages/@react-spectrum/numberfield/test/NumberField.test.js

adobe-bot · 2020-10-02T21:39:53Z

Build successful! 🎉

move focus from stepper buttons back to the input when they are pressed

…layed in the input

majornista

NumberField does not announce value change when either the increment or decrement value is pressed. This is because the NumberField is not considered focused when either the Increment or Decrement button has focus:

See:

react-spectrum/packages/@react-aria/spinbutton/src/useSpinButton.ts

Lines 112 to 124 in 0ac271a

    
           let onFocus = () => { 
        
             isFocused.current = true; 
        
           }; 
        
           let onBlur = () => { 
        
             isFocused.current = false; 
        
           }; 
        
           useEffect(() => { 
        
             if (isFocused.current) { 
        
               announce(textValue || `${value}`); 
        
             } 
        
           }, [textValue, value]);

Should the Increment and Decrement buttons also have onFocus and onBlur handlers?

MouseDown on the Increment or Decrement button should shift focus to the textfield, but on a mobile device, TouchStart should keep focus on the Increment or Decrement button. With this behavior and live region announcement on input, a mobile screen reader user can set focus to increment or decrement button, and continue pressing the button, receiving feedback for each value change, without focus shifting to the input and opening the on screen keyboard.

it now reads 'Empty' instead of 50% for an empty field, this is a POC

adobe-bot · 2020-10-09T00:09:00Z

Build successful! 🎉

move translation string into aria

adobe-bot · 2020-10-09T00:23:34Z

Build successful! 🎉

FF should support everything we want Safari has a decimal point on iOS

adobe-bot · 2020-10-12T21:01:01Z

Build successful! 🎉

adobe-bot · 2020-10-12T21:11:22Z

Build successful! 🎉

Add types Remove a few options from some of our API's to reduce surface area Separate rounding from clamping Simplify rounding via the formatter

adobe-bot · 2020-12-15T02:24:36Z

Build successful! 🎉

Fix spinning when starting with a not undefined min/max Fix tests Only parse when the actual input or formatter (parser) changes

adobe-bot · 2021-01-05T01:22:52Z

Build successful! 🎉

adobe-bot · 2021-01-05T01:47:25Z

Build successful! 🎉

adobe-bot · 2021-01-06T00:28:24Z

Build successful! 🎉

adobe-bot · 2021-01-06T00:34:59Z

Build successful! 🎉

packages/@react-aria/numberfield/src/useNumberField.ts

packages/@react-stately/numberfield/src/useNumberFieldState.ts

adobe-bot · 2021-01-08T00:20:51Z

Build successful! 🎉

adobe-bot · 2021-01-08T02:10:24Z

Build successful! 🎉

adobe-bot · 2021-01-11T23:57:48Z

Build successful! 🎉

devongovett · 2021-01-15T23:03:36Z

packages/@react-spectrum/numberfield/src/NumberField.tsx

-  let state = useNumberFieldState(props);
+
+  let {locale} = useLocale();
+  let [currentNumeralSystem, setCurrentNumeralSystem] = useState<NumeralSystem | undefined>();


Should this be in stately? Seems weird to pass it around as a prop, especially the setter.

So reason for this is that I need access to the current numeral system before I useNumberFormatter, which I have to create in NumberField and pass into the state

devongovett · 2021-01-15T23:07:03Z

packages/@react-stately/numberfield/src/useNumberFieldState.ts

+  textValue?: string
+}
+
+// I don't know how to remove this one so it's not duplicated and not being imported from aria, should it be in react-types? shared?


Yeah should go in types. I guess shared is fine? Otherwise we'd need to create a whole new i18n types package... we do have a locale.d.ts file in there already 🤔

devongovett · 2021-01-15T23:30:35Z

FYI pinged about a couple old unresolved threads too in case you don't see them :)

Happy for them to be follow ups if you want but just let me know

# Conflicts: # packages/@react-stately/color/src/useHexColorFieldState.ts

adobe-bot · 2021-01-16T00:46:28Z

Build successful! 🎉

…ly live in stately yet

adobe-bot · 2021-01-16T01:23:12Z

Build successful! 🎉

adobe-bot · 2021-01-20T18:37:51Z

Build successful! 🎉

snowystinger added 4 commits August 11, 2020 09:56

WIP Numberfield

05fc5ca

Mostly a save point, but this starts to address style issues Labels the field correctly Adds some stories Moves specs because it's more complicated and easier to read on its own

Rewrote a lot of the css, more will probably be needed as this doesn'…

19f3af5

…t cover active/hover states buttons are now on the right side

fix test and add one for invalid

c07e926

snowystinger requested review from dannify and devongovett as code owners August 28, 2020 19:18

Merge branch 'main' into numberfield-followup

b983837

snowystinger added 3 commits August 28, 2020 14:43

Add labels to stories

1766699

Fix labels Add label test

reset jest exclude rule

0d739e2

fix lint

15c91e2

emircado mentioned this pull request Sep 4, 2020

Create HexColorField component #986

Merged

5 tasks

Merge branch 'main' into numberfield-followup

b855801

# Conflicts: # packages/@react-aria/numberfield/src/useNumberField.ts # packages/@react-spectrum/numberfield/test/NumberField.test.js

snowystinger added 2 commits October 2, 2020 17:08

fixed remaining styles

77c59af

move focus from stepper buttons back to the input when they are pressed

round numbers so that onChange fires with the same value that is disp…

600aff7

…layed in the input

majornista requested changes Oct 5, 2020

View reviewed changes

snowystinger added 3 commits October 5, 2020 14:09

Merge branch 'main' into numberfield-followup

9b30515

Supporting VO, it now reads the number when the stepper buttons are used

36ad251

it now reads 'Empty' instead of 50% for an empty field, this is a POC

fix lint

9d762e2

fix dependencies

c5c5238

move translation string into aria

snowystinger added 3 commits October 12, 2020 13:21

Borrow some code about mobile experiences

3c75d53

FF should support everything we want Safari has a decimal point on iOS

fix lint and TS

7aa15c4

Merge branch 'main' into numberfield-followup

d629112

Fix Firefox style for stepper buttons getting hidden

0fa0ccf

Move clean into the parser

f0caddf

Add types Remove a few options from some of our API's to reduce surface area Separate rounding from clamping Simplify rounding via the formatter

Rename isChained to isSpinning to clarify role of variable

37e65df

Fix spinning when starting with a not undefined min/max Fix tests Only parse when the actual input or formatter (parser) changes

add back focus tracking logic until something better can be done

fdf3b56

snowystinger added 2 commits January 5, 2021 16:15

Stop focusing the input when the steppers are pressed

426f848

Merge branch 'main' into numberfield-followup

8b05092

devongovett reviewed Jan 7, 2021

View reviewed changes

packages/@react-aria/numberfield/src/useNumberField.ts Outdated Show resolved Hide resolved

packages/@react-stately/numberfield/src/useNumberFieldState.ts Outdated Show resolved Hide resolved

Splitting out into other PR's and general CSS cleanup

e6c8eb0

Remove need to add 'isSpinning' to our API

a866393

snowystinger added 2 commits January 8, 2021 16:49

Move aria out of stately for numberfield

9801a9d

fix import

138489b

devongovett reviewed Jan 15, 2021

View reviewed changes

Merge branch 'main' into numberfield-followup

6aa9a19

# Conflicts: # packages/@react-stately/color/src/useHexColorFieldState.ts

Move types around and set state through stately, even if it can't ful…

fe5f063

…ly live in stately yet

Merge Main

b835503

devongovett approved these changes Jan 20, 2021

View reviewed changes

devongovett merged commit bd1afa1 into main Jan 20, 2021

devongovett deleted the numberfield-followup branch January 20, 2021 19:03

snowystinger mentioned this pull request Jan 20, 2021

Numberfield further work #1486

Closed

12 tasks

	let onFocus = () => {
	isFocused.current = true;
	};

	let onBlur = () => {
	isFocused.current = false;
	};

	useEffect(() => {
	if (isFocused.current) {
	announce(textValue \|\| `${value}`);
	}
	}, [textValue, value]);

Numberfield followup #1029

Numberfield followup #1029

Uh oh!

Conversation

snowystinger commented Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

adobe-bot commented Aug 28, 2020

Uh oh!

adobe-bot commented Aug 31, 2020

Uh oh!

adobe-bot commented Oct 2, 2020

Uh oh!

majornista left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adobe-bot commented Oct 9, 2020

Uh oh!

adobe-bot commented Oct 9, 2020

Uh oh!

adobe-bot commented Oct 12, 2020

Uh oh!

adobe-bot commented Oct 12, 2020

Uh oh!

adobe-bot commented Dec 15, 2020

Uh oh!

adobe-bot commented Jan 5, 2021

Uh oh!

adobe-bot commented Jan 5, 2021

Uh oh!

adobe-bot commented Jan 6, 2021

Uh oh!

adobe-bot commented Jan 6, 2021

Uh oh!

Uh oh!

Uh oh!

adobe-bot commented Jan 8, 2021

Uh oh!

adobe-bot commented Jan 8, 2021

Uh oh!

adobe-bot commented Jan 11, 2021

Uh oh!

devongovett Jan 15, 2021

Choose a reason for hiding this comment

Uh oh!

snowystinger Jan 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devongovett Jan 15, 2021

Choose a reason for hiding this comment

Uh oh!

devongovett commented Jan 15, 2021

Uh oh!

adobe-bot commented Jan 16, 2021

Uh oh!

adobe-bot commented Jan 16, 2021

Uh oh!

adobe-bot commented Jan 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

snowystinger commented Aug 28, 2020 •

edited

Loading

majornista left a comment •

edited

Loading

snowystinger Jan 16, 2021 •

edited

Loading