-
Notifications
You must be signed in to change notification settings - Fork 15
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
[FX-5908] Fix initial position of Slider #4572
Conversation
🦋 Changeset detectedLatest commit: 36bd2a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 80 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
5f1400c
to
ea55f5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works nice. However, some changes seem unnecessary. Maybe it is some kind of leftover, or I am missing something. Lets double check.
@toptal-anvil ping reviewers |
391043b
to
7ba8dfd
Compare
@TomasSlama is it intentional that the label is shifted to right instead of being centered as before? |
it is not, thank you for noticing, I will take a look |
@toptal-anvil ping reviewers |
d52b73e
to
6ce0bf4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
Please check this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, compared both staging and temploy, works as expected!
77d3b7b
to
36bd2a8
Compare
Merging anyway, as the problem is only with docs deployment, so CI issue
|
FX-5908
Description
Fix initial position of Slider when rendered directly on visible screen.
useOnScreen
is returningfalse
by default, so before theIntersectionObserver
actually starts observing, the tooltip blinks from the bottom to the top. I enhanced the hook to also returnisObserved
, so the component can check if the returned value is correct.How to test
tooltip='on'
Screenshots
Development checks
picasso-tailwind-merge
requires major update (check itsREADME.md
)props
in component with documentationexamples
for componentBreaking change
PR commands
List of available commands:
@toptal-bot run package:alpha-release
- Release alpha version@toptal-anvil ping reviewers
- Ping FX team for reviewPR Review Guidelines
When to approve? ✅
You are OK with merging this PR and
nit:
to your comment. (ex.nit: I'd rename this variable from makeCircle to getCircle
)When to request changes? ❌
You are not OK with merging this PR because
When to comment (neither ✅ nor ❌)
You want your comments to be addressed before merging this PR in cases like:
How to handle the comments?