fix: sliders mouseup is not firing when leaving the window #5184
Conversation
This file contains 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
….com/microsoft/fast into users/robarb/tests-horizontal-scroll
…/robarb/tests-horizontal-scroll
….com/microsoft/fast into users/robarb/tests-horizontal-scroll
…/robarb/slider-mouseup
robarbms
requested review from
chrisdholt,
EisenbergEffect,
janechu and
nicholasrice
as code owners
scomea
reviewed
Sep 14, 2021
|
are the horizonal scroll changes related to the slider issue? |
# Pull Request ## 📖 Description Adding the file component to creator as a StandardControlPlugin for the "src" attribute. This replaces the image src text box in the right side form with a File component button configured to allow image file types (.jpg, .png, .gif). Clicking the button will open a file picker and when an image is selected it is converted to an ObjectUrl which is used for the image src. *Note: Images added in this way are stored only on the client and only last for the session. No data about the file is transmitted or available outside of the client nor does it utilize cookies in any way. <!--- Provide some background and a description of your work. What problem does this change solve? Is this a breaking change, chore, fix, feature, etc? --> ### 🎫 Issues <!--- * List and link relevant issues here. --> ## 👩💻 Reviewer Notes <!--- Provide some notes for reviewers to help them provide targeted feedback and testing. Do you recommend a smoke test for this PR? What steps should be followed? Are there particular areas of the code the reviewer should focus on? --> ## 📑 Test Plan <!--- Please provide a summary of the tests affected by this work and any unique strategies employed in testing the features/fixes. --> ## ✅ Checklist ### General <!--- Review the list and put an x in the boxes that apply. --> - [ ] I have included a change request file using `$ yarn change` - [ ] I have added tests for my changes. - [x] I have tested my changes. - [ ] I have updated the project documentation to reflect my changes. - [x] I have read the [CONTRIBUTING](https://github.com/Microsoft/fast/blob/master/CONTRIBUTING.md) documentation and followed the [standards](https://www.fast.design/docs/community/code-of-conduct/#our-standards) for this project. ### Component-specific <!--- Review the list and put an x in the boxes that apply. --> <!--- Remove this section if not applicable. --> - [ ] I have added a new component - [ ] I have modified an existing component - [ ] I have updated the [definition file](https://github.com/Microsoft/fast/blob/master/packages/web-components/fast-components/CONTRIBUTING.md#definition) - [ ] I have updated the [configuration file](https://github.com/Microsoft/fast/blob/master/packages/web-components/fast-components/CONTRIBUTING.md#configuration) ## ⏭ Next Steps <!--- If there is relevant follow-up work to this PR, please list any existing issues or provide brief descriptions of what you would like to do next. -->
…ssing values are not hidden or missed (#5030) * remove css color assignment from body of storybook to ensure there missing values are not hidden or missed * Change files
* chore: eslint fixes in fast-components * Change files Co-authored-by: Chris Holt <chhol@microsoft.com>
added 2 commits
September 15, 2021 08:19
…/robarb/slider-mouseup
…osoft/fast into users/robarb/slider-mouseup
nicholasrice
approved these changes
Sep 15, 2021
nicholasrice
suggested changes
Sep 15, 2021
added 4 commits
September 15, 2021 13:37
…/robarb/slider-mouseup
…osoft/fast into users/robarb/slider-mouseup
Another PR got merged some how. I've undone the errant changes |
Fixed this. I've removed the horizontal-scroll changes that some how got merged into this PR. |
chrisdholt
approved these changes
Sep 16, 2021
|
@robarbms FYI - looks like there are a couple of conflicts with the Horizontal Scroll changes going in. |
There was a whitespace conflict 😑 |
|
It looks like git history got messed up on this, would you mind resolving prior to merge? |
| this.value = `${this.calculateNewValue(controlValue)}`; | ||
| /** | ||
| * | ||
| * @param e - MouseEvent or null. If there is no event handler it will remove the events |
There was a problem hiding this comment.
Suggested change
| * @param e - MouseEvent or null. If there is no event handler it will remove the events | |
| * @param e - MouseEvent or null. If null is provided, events handlers will be removed. |
nicholasrice
approved these changes
Sep 28, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pull Request
📖 Description
When leaving the window, the slider will not register a mouseup event.
🎫 Issues
#4825
👩💻 Reviewer Notes
I'm capturing mouseleave on the window document. I found that mouseout fired while still in the window. Perhaps it's firing for the shadow DOMs mouseout? The window element wouldn't fire on mouseleave so I'm using window.document. I attach it on mousedown and remove it on mouseup per the existing pattern.
📑 Test Plan
✅ Checklist
General
$ yarn changeComponent-specific
⏭ Next Steps