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.

Sign up for GitHub

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

Jump to bottom

feat(Slider): add tooltip on active thumb button #1529

Merged
merged 1 commit into from
Sep 13, 2022
Merged

feat(Slider): add tooltip on active thumb button #1529

merged 1 commit into from
Sep 13, 2022

Conversation

tujoworker
Copy link
Member

@tujoworker tujoworker commented Aug 25, 2022 •
edited
Loading

Its optional to show a Tooltip. To show it, use the tooltip property. Also, alwaysShowTooltip will ensure, it always is visible, and not only on mouseOver, touchStart and focus.

Check out the docs and the demos.

Here is a CSB and a 👉 demo to test out.

  • Should we only show a toolbar, if a formatting is given? Or should it be prop based? tooltip={true}

Relies on PR #1547

Screenshot 2022-08-25 at 12 34 35

Screenshot 2022-08-25 at 12 46 14

Screenshot 2022-08-25 at 12 50 50

@tujoworker tujoworker force-pushed the feat/slider-toolbar branch from 93dcc21 to c1de47a Compare August 25, 2022 10:18
@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 25, 2022 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d68955e:

Sandbox Source
eufemia-starter Configuration
eufemia-slider-tooltip PR

@tujoworker tujoworker force-pushed the feat/slider-toolbar branch from c1de47a to 7adce08 Compare August 25, 2022 10:38
@gatsby-cloud
Copy link

gatsby-cloud bot commented Aug 25, 2022 •
edited
Loading

✅ DNB Eufemia Portal deploy preview ready

@tujoworker tujoworker force-pushed the feat/slider-toolbar branch from 7adce08 to 9cd1721 Compare August 25, 2022 11:06
@tujoworker tujoworker changed the title feat(Slider): add toolbar on active thumb button feat(Slider): add tooltip on active thumb button Aug 26, 2022
@tujoworker tujoworker force-pushed the feat/slider-toolbar branch 6 times, most recently from 8fd28ec to e15203e Compare September 1, 2022 09:30
@tujoworker tujoworker marked this pull request as ready for review September 1, 2022 09:30
@tujoworker tujoworker force-pushed the feat/slider-toolbar branch 2 times, most recently from e7aaa14 to af468de Compare September 1, 2022 10:43
@tujoworker tujoworker requested a review from langz September 1, 2022 11:19
@tujoworker tujoworker force-pushed the feat/slider-toolbar branch 2 times, most recently from d1c325b to fa5b315 Compare September 1, 2022 11:35
@langz
Copy link
Contributor

langz commented Sep 1, 2022 •
edited
Loading

I'll take a look at this, and test it a bit, when I get time tomorrow 🕙
Looks cool ⭐

@langz
Copy link
Contributor

langz commented Sep 2, 2022 •
edited
Loading

A small nitpick that I saw when using my iPhone, both Safari and Chrome.

There's a small gap in the tooltip, between the "circle" and the "arrow", but it only happens at certain times/values:

302345667_859426375038146_4013779052565230971_n

302283051_801310464399567_2180611951952021715_n

@tujoworker
Copy link
Member Author

yeah, I agree – we should fix this in another PR 👍

@langz
Copy link
Contributor

langz commented Sep 2, 2022 •
edited
Loading

Some other thoughts 💭

I can see that on mobile, we are able to click/drag the tool tip, which will also move/drag the slider.
Should it be like that, or would it be better that clicking on the tooltip rather make it possible to select the text in the tooltip, like here https://eufemia-featslidertoolbar.gtsb.io/uilib/components/tooltip/#button-with-active-tooltip ?

Do we have to consider what will happen if the tooltip has more text than what fits in the container/screen size? Will the text in the tooltip break into multiple lines?

langz
langz approved these changes Sep 2, 2022
View reviewed changes
Copy link
Contributor

@langz langz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

@tujoworker
Copy link
Member Author

Should it be like that, or would clicking on the tooltip rather make it possible to select the text in the tooltip

Yeah, nice finding! And yes, we should change that and make it select able.

When it comes to line wrapping, I will have a look at it as well.

@tujoworker
Copy link
Member Author

Regarding the wrapping. Because of UX, we should not wrap numbers. And because the Slider, in theory, only will show numbers, I think, its OK to have nowrap for now.
I made some changes to the Tooltip behavior. Now the text is selectable, but there are some small issues with the position. We need to fix this in the Tooltip component in another PR later.

@tujoworker tujoworker force-pushed the feat/slider-toolbar branch 3 times, most recently from 6f6f9db to d819abd Compare September 5, 2022 10:40
@tujoworker tujoworker force-pushed the feat/slider-toolbar branch 8 times, most recently from dc2bb63 to fadc36a Compare September 6, 2022 09:16
@tujoworker tujoworker merged commit 437f81c into main Sep 13, 2022
@tujoworker tujoworker deleted the feat/slider-toolbar branch September 13, 2022 07:10
tujoworker pushed a commit that referenced this pull request Oct 3, 2022
@semantic-release-bot
chore(release): 9.31.0 [skip ci]
2d48d4f 
# [9.31.0](v9.30.0...v9.31.0) (2022-10-03)

### Bug Fixes

* **Accordion:** support nested accordions ([#1595](#1595)) ([dc14a79](dc14a79))
* **AnimateHeight:** [internal] rewrite to TypeScript ([#1570](#1570)) ([e2f0f0d](e2f0f0d))
* **Autocomplete:** ensure value is not visible behind the trigger button ([#1543](#1543)) ([de65acb](de65acb))
* **Autocomplete:** make DrawerList direction observer work ([#1535](#1535)) ([fcdf9f8](fcdf9f8))
* **Autocomplete:** touch device issue: ensure focus is set after second input focus ([#1540](#1540)) ([2f3b82e](2f3b82e))
* **Avatar:** don't overwrite SVG color ([#1579](#1579)) ([a6b3f50](a6b3f50))
* **DrawerList:** remove unused white area on the right side ([#1542](#1542)) ([b5575e7](b5575e7)), closes [#1531](#1531)
* **Input:** ensure dnb-input--null class will not be set ([#1544](#1544)) ([885d2d1](885d2d1))
* **Modal:** Safari Desktop fullscreen video issue ([#1582](#1582)) ([5219ccd](5219ccd))
* **PaginationInfinity:** ensure the load button does not appear when current_page decreases ([#1147](#1147)) ([e19a377](e19a377))
* **Section:** fix spacing + rewrite to TypeScript ([#1573](#1573)) ([4352495](4352495))
* **Slider:** enhance Safari (desktop) UX ([#1539](#1539)) ([6ca785f](6ca785f))
* **Slider:** make it optional to provide an updated prop value ([#1537](#1537)) ([ff1f3b7](ff1f3b7))
* **StepIndicator:** change chevron icon to pointing to the right ([#1541](#1541)) ([8529d8c](8529d8c))
* **Tooltip:** convert to camelCase props with backwards compatibility ([#1557](#1557)) ([24285cb](24285cb))
* **Tooltip:** convert to TypeScript ([#1549](#1549)) ([9789ec6](9789ec6))
* **Tooltip:** ensure controlled active prop takes presence ([#1547](#1547)) ([ac28883](ac28883)), closes [#1411](#1411)
* **Tooltip:** fix React Portal handling ([#1588](#1588)) ([26f4c61](26f4c61))
* **Tooltip:** merge style property with internal ([#1591](#1591)) ([b3e3901](b3e3901))
* **Tooltip:** refactor tests from Enzyme to TestingLib ([#1553](#1553)) ([dde8576](dde8576))
* **Tooltip:** remove unused FormRow integration ([#1589](#1589)) ([be37918](be37918))
* **Tooltip:** rewrite to functional components with React Hooks ([#1555](#1555)) ([8b04fc2](8b04fc2))
* **Tooltip:** use Eufemia cubic-bezier for animations ([#1552](#1552)) ([c60b3a6](c60b3a6))

### Features

* **Breadcrumb:** add animation when in collapse mode ([#1563](#1563)) ([ded90c2](ded90c2))
* **Breadcumb:** align spacing and add small, medium and large ([#1574](#1574)) ([cf4c312](cf4c312))
* **DefinitionList:** add horizontal direction (inline) support ([#1536](#1536)) ([59ec706](59ec706))
* **Easing:** expose default CSS easing with --easing-default ([#1562](#1562)) ([c18b021](c18b021))
* **HeightAnimation:** add new height animation component ([#1566](#1566)) ([72b1da5](72b1da5))
* **HeightAnimation:** add onInit to get animation instance ([#1597](#1597)) ([bf4e656](bf4e656))
* **HeightAnimation:** adjust height with animation when content changes ([#1569](#1569)) ([f0779c2](f0779c2))
* **HelpButton:** rewrite to TypeScript ([#1565](#1565)) ([d4f26c3](d4f26c3))
* **icons:** add "file_jpg, file_png, fish, newspaper, sort" icon ([#1575](#1575)) ([bf3769f](bf3769f))
* **InteractionInvalidation:** add options for partial invalidation (tabIndex and ariaHidden) ([#1559](#1559)) ([6cfc235](6cfc235))
* **Logo:** add inherit_color prop ([#1578](#1578)) ([0983343](0983343))
* **Slider:** add tooltip on active thumb button ([#1529](#1529)) ([437f81c](437f81c))
* **Tooltip:** add skip_portal property ([#1545](#1545)) ([7f492a5](7f492a5))
* **useMediaQuery:** add disable as an option ([#1572](#1572)) ([6078cb4](6078cb4))
@tujoworker
Copy link
Member Author

🎉 This PR is included in version 9.31.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@langz langz langz approved these changes

Assignees
No one assigned
Labels
released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tujoworker @langz