Skip to content
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

fix: keep tooltip align with handle when dragging #696

Merged
merged 6 commits into from
Oct 30, 2020
Merged

Conversation

kerm1it
Copy link
Member

@kerm1it kerm1it commented Oct 15, 2020

close #693

@vercel
Copy link

vercel bot commented Oct 15, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/react-component/slider/5ptjlu2yu
✅ Preview: https://slider-git-fix-tooltip-follow.react-component.vercel.app

@kerm1it
Copy link
Member Author

kerm1it commented Oct 15, 2020

@zombieJ 等预览部署好了,看看 handle 那个例子。

@codecov
Copy link

codecov bot commented Oct 15, 2020

Codecov Report

Merging #696 into master will increase coverage by 0.26%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #696      +/-   ##
==========================================
+ Coverage   88.26%   88.52%   +0.26%     
==========================================
  Files          10       11       +1     
  Lines         690      706      +16     
  Branches      178      179       +1     
==========================================
+ Hits          609      625      +16     
  Misses         81       81              
Impacted Files Coverage Δ
src/index.tsx 100.00% <ø> (ø)
src/common/SliderTooltip.tsx 100.00% <100.00%> (ø)
src/createSliderWithTooltip.tsx 96.15% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e793107...cb16620. Read the comment docs.

@zombieJ
Copy link
Member

zombieJ commented Oct 18, 2020

的确蛋疼……

@zombieJ
Copy link
Member

zombieJ commented Oct 18, 2020

加了一个 nowrap,看看如何?

@kerm1it
Copy link
Member Author

kerm1it commented Oct 19, 2020

这样可以,但是觉得不太优雅😂

不能让用户去这样写吧?

@zombieJ
Copy link
Member

zombieJ commented Oct 19, 2020

嗯,这个遇到 overflow: hidden 的情况也会被截掉。感觉还是应该放最外面。如果是这样的话,应该是每次 value 变化,就触发一下 Tooltip 的 forceAlignkeepAlign 那个废弃掉,以前实现的相当不优雅。

@kerm1it
Copy link
Member Author

kerm1it commented Oct 19, 2020

这样也会有一个问题,因为 slider 默认是没有 tooltip 包裹的,就例如 handle 的那个例子,这样就需要让用户自己去实现这个逻辑,所以可能需要我们实现 slider 的时候,默认用 tooltip 包裹。

@zombieJ
Copy link
Member

zombieJ commented Oct 19, 2020

这样也会有一个问题,因为 slider 默认是没有 tooltip 包裹的,就例如 handle 的那个例子,这样就需要让用户自己去实现这个逻辑,所以可能需要我们实现 slider 的时候,默认用 tooltip 包裹。

rc-slider 里 demo 更新一下就行了,它比较裸用户可以随便拓展。 antd 封装了就成。

@kerm1it
Copy link
Member Author

kerm1it commented Oct 19, 2020

这样也会有一个问题,因为 slider 默认是没有 tooltip 包裹的,就例如 handle 的那个例子,这样就需要让用户自己去实现这个逻辑,所以可能需要我们实现 slider 的时候,默认用 tooltip 包裹。

rc-slider 里 demo 更新一下就行了,它比较裸用户可以随便拓展。 antd 封装了就成。

那其实 antd 不用动,只需要更新一些 rc-slider 的 demo 就行了,没有什么功能性的改动?

@zombieJ
Copy link
Member

zombieJ commented Oct 19, 2020

那就 ok 了,改 demo~~

@kerm1it
Copy link
Member Author

kerm1it commented Oct 19, 2020

@zombieJ 最后还是和 antd 一样加了个 SliderTooltip 组件,因为发现有个 createSliderWithTooltip 方法,这样以来通过这个方法创建的 Slider 和 Range 都会有 自动对齐的功能

而且用户如果想自定义的话可以通过 SliderTooltip 组件自定义实现。

@kerm1it
Copy link
Member Author

kerm1it commented Oct 27, 2020

@zombieJ 我改了一下,同时监听 visibleoverlay ,取消了循环。

@kerm1it kerm1it requested a review from zombieJ October 29, 2020 11:58
src/index.tsx Outdated Show resolved Hide resolved
@zombieJ
Copy link
Member

zombieJ commented Oct 30, 2020

弄完我发个 minor 版本~

@kerm1it kerm1it requested a review from zombieJ October 30, 2020 07:13
@zombieJ zombieJ merged commit 7b39ad1 into master Oct 30, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-tooltip-follow branch October 30, 2020 07:38
@zombieJ
Copy link
Member

zombieJ commented Oct 30, 2020

+ rc-slider@9.6.0

@zombieJ
Copy link
Member

zombieJ commented Oct 30, 2020

antd 也可以 bump 一下~~

@kerm1it
Copy link
Member Author

kerm1it commented Oct 31, 2020

antd 那边暂时不升级了,因为那边的 SliderTooltip 使用的 Tooltip 是 antd 里面的,这边的还不能复用呢。

@zombieJ
Copy link
Member

zombieJ commented Nov 2, 2020

不用这个 Tooltip,只是版本对齐一下。免得以后加新 feature 突然跳 2 个 minor 懵逼了~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tooltip does not follow handle
2 participants