-
Notifications
You must be signed in to change notification settings - Fork 80
feat: add useValue controller #12670
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
Draft
eriklharper
wants to merge
78
commits into
dev
Choose a base branch
from
eriklharper/12278-useValue-controller
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lharper/12278-useValue-controller
…lharper/12278-useValue-controller
…le change, input and direct value setting. Takes component events as arguments and handles emitting logic. Need to handle default prevention and update the direct setting case in input-text.
…lharper/12278-useValue-controller
…12391 without having to call preventDefault on the change event. Add userChangedValue logic to controller to support direct value setting on the component properly.
…lharper/12278-useValue-controller
…s to be more descriptive
…ing the direct value change logic in hostUpdate.
…lharper/12278-useValue-controller
…d to null or undefined
…at the controller handles this
…g the first step at creating a getValidNumberValue and getLocalizedNumberString function
…lharper/12278-useValue-controller
…with 2 new methods for getting the new validated value and localized value based on that. will look at addressing the broken tests as a result of this change
…umber type. Renaming hasLeadingZeros to getLeadingZeros since it returns an array from the regex match results instead of a boolean.
…ctly set right after a user input event. This was caused by the userChangedValue staying set to true after the value was committed. Now when inputValue and commitValue methods are called, userChangedValue is immediately set back to false right after updating the component's value so that the hostUpdate and willUpdate lifecycle methods will know exactly when the user actually modified the value.
…irectValueChange function to fire incorrectly during a user input event. This is making me think that perhaps instead of lastEmittedValue we should make a lastCommittedValue that only gets set in commitValue so that when a direct value change happens it is more distinguishable.
…lharper/12278-useValue-controller
…, "-", "e" or "E" when all the text is selected in the input.
…lharper/12278-useValue-controller
…lharper/12278-useValue-controller
…e best starting point. Many failed tests to fix still, but the general approach is to sync up the selectedItems property right before committing the value with the controller. The selectedItems setter will no longer update the value because we need the controller to handle that.
…lharper/12278-useValue-controller
…lharper/12278-useValue-controller
…alue to accommodate a custom name for combobox-item and potentially others...
…lharper/12278-useValue-controller
…after setting the component's value. I thought this needed to happen, but because there's no property watcher that fires immediately after the value is set like in Stencil, the more appropriate place to reset it back to false is in hostUpdate after handling the direct value change logic.
…w the last 2 remaining failed combobox tests are back from before.
…lharper/12278-useValue-controller
…ittedValue to empty string when the value is falsy.
…hangedValue to valueSetDirectly. Because direct setting of a value property like "selected" for comobobox-item will happen from the parent combobox when the change was initiated by the user via a click or keyboard commit, the name "userChangedValue" doesn't make sense in this case. Tracking whether the property was directly modified or not will always apply, even if that happens as a downstream result of a user action. We'll need to come up with another code pathway to communicating between the parent <> child in a case like this to update parent state when a child is directly changed NOT from user input.
…their parent when their selected property is directly changed. Need to add a test that directly sets selected on a combobox-item when there is already another one selected.
… event once because the 2nd commit action doesn't actually change the value
…g on initial mount
…lharper/12278-useValue-controller
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
Issues tied to a new feature or request.
refactor
Issues tied to code that needs to be significantly reworked.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issue: #12278
Summary
This PR introduces a
useValue
controller for handling direct value changes and managinginput
andchange
event emitting behavior for form components that implement avalue
property.useValue
exposes the following API:inputValue({ inputEventEmitter: EventEmitter, value: string })
method that takes the component's custominput
EventEmitter instance and the value to input. This will trigger the component'sinput
event and should be called in response to direct user input, via keyboard and mouse events.useValue
handles setting the component'svalue
property and emitting theinput
event.commitCurrentValue({ changeEventEmitter: EventEmitter })
method that takes the component's customchange
EventEmitter instance and handles committing the component's currently set value. This method should be called when theEnter
key is pressed while the component has focus and when the component loses focus (blurred).useValue
will emit the component'schange
event only if the component's currentvalue
differs fromuseValue
's internally-managedlastCommittedValue
.commitValue({ changeEventEmitter: EventEmitter, value: string })
method that takes the component's customchange
EventEmitter instance and thevalue
to commit.useValue
will emit the component'schange
event only if the suppliedvalue
differs fromuseValue
's internally-managedlastCommittedValue
. This method should be called on Enter keypress or blur and only when there is a need to pass a custom value that is not already set on the component, such as a reset or clear event where the new value is being set to""
or needs to be restored to a different value, or if the value needs to be validated prior to committing.useValue
handles default event prevention for the component'sinput
andchange
events, restoring the component'svalue
to its internally-trackedpreviousValue
property.useValue
also knows not to emit anyinput
orchange
events when the value is directly modified on a component instance as long as only the above methods are utilized to modify the value in direct response to user input.