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 multiThumbBehavior property #1526

Merged
merged 1 commit into from
Aug 25, 2022
Merged

feat(Slider): add multiThumbBehavior property #1526

merged 1 commit into from
Aug 25, 2022

Conversation

tujoworker
Copy link
Member

@tujoworker tujoworker commented Aug 24, 2022 •
edited
Loading

In addition to the new property;

  • Add a test + some more
  • Add type docs
slider.swap.mov

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 24, 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 cbc15ce:

Sandbox Source
eufemia-starter Configuration

@gatsby-cloud
Copy link

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

Gatsby Cloud Build Report

DNB Eufemia Portal

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 9m

Performance

Lighthouse report

Metric Score
Performance 🔶 69
Accessibility 💚 100
Best Practices 💚 100
SEO 💚 92

🔗 View full report

@tujoworker tujoworker requested a review from langz August 25, 2022 06:14
@tujoworker tujoworker force-pushed the feat/slider-omitThumbSwap branch 3 times, most recently from 15c176f to fa7670e Compare August 25, 2022 07:27
@ahmetacer5
Copy link

ahmetacer5 commented Aug 25, 2022 •
edited
Loading

In addition to the new property;

  • Add a test + some more
  • Add type docs

slider.swap.mov

This is great but should we allow thumbs to push each other when omitThumbSwap is enabled?

Screen.Recording.2022-08-25.at.09.29.45.mov

@tujoworker
Copy link
Member Author

What you think about this behaviour?

yeah, we may add this as well. Do you have any property name recommendation? Or should it do so as default? It differs if the use-case I would say, so both can be interesting. Probably as a separate feature, or as an alternative:

omitThumbSwap: 'push'

@tujoworker tujoworker force-pushed the feat/slider-omitThumbSwap branch from fa7670e to 2e5184d Compare August 25, 2022 07:41
@ahmetacer5
Copy link

What you think about this?
thumbBehaviour: 'omit' | 'push' | 'swap' = 'push'

@tujoworker
Copy link
Member Author

Yeah, we can do that 👍

Regarding the push behavior – I see, material UI is recommending doing it in your app instead. But the code looks so terrible to have in your app code, just to get the push behavior 🤦‍♂

Screenshot 2022-08-25 at 09 44 15

️

langz
langz approved these changes Aug 25, 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.

Nice feature 💯
If we think this will be used a lot, it could be beneficial to add an example to the docs, so that it will be easier to see/find out that we have this feature for devs.

@ahmetacer5
Copy link

Yea I agree better to have the logic within the component.

@tujoworker
Copy link
Member Author

tujoworker commented Aug 25, 2022 •
edited
Loading

it could be beneficial to add an example to the docs

yeah, made an example there as well now 👍

slider-behavior.mov

Here are the changes: one and two

@tujoworker tujoworker force-pushed the feat/slider-omitThumbSwap branch from 2e5184d to 294d54d Compare August 25, 2022 08:32
@tujoworker
feat(Slider): add omitThumbSwap property
cbc15ce 
In addition to the new property;

- Add a test + some more
- Add type docs
@tujoworker tujoworker force-pushed the feat/slider-omitThumbSwap branch from 294d54d to cbc15ce Compare August 25, 2022 08:48
@tujoworker tujoworker changed the title feat(Slider): add omitThumbSwap property feat(Slider): add multiThumbBehavior property Aug 25, 2022
@tujoworker tujoworker merged commit f835651 into main Aug 25, 2022
@tujoworker tujoworker deleted the feat/slider-omitThumbSwap branch August 25, 2022 09:24
tujoworker pushed a commit that referenced this pull request Aug 29, 2022
@semantic-release-bot
chore(release): 9.30.0 [skip ci]
50e4faa 
# [9.30.0](v9.29.0...v9.30.0) (2022-08-29)

### Bug Fixes

* **Slider:** fix reverse with min and max defiend ([#1533](#1533)) ([6c169b4](6c169b4))
* **Slider:** prevent onChange being called with same value ([#1528](#1528)) ([115b056](115b056))
* **Slider:** use numbers instead of css reverse when reversing slider ([#1532](#1532)) ([e2e83a0](e2e83a0))
* **Theme:** correct DNB Eiendom state colors mint-green-50 to pistachio ([#1527](#1527)) ([b7f532e](b7f532e))
* **withCamelCaseProps:** make exception for className ([#1534](#1534)) ([a0a0082](a0a0082))

### Features

* **Slider:** add multiThumbBehavior property ([#1526](#1526)) ([f835651](f835651))
@tujoworker
Copy link
Member Author

🎉 This PR is included in version 9.30.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.
3 participants
@tujoworker @ahmetacer5 @langz