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

Mj.dual range slider #1148

Merged
merged 61 commits into from
Jul 17, 2023
Merged

Mj.dual range slider #1148

merged 61 commits into from
Jul 17, 2023

Conversation

micahjones13
Copy link
Collaborator

@micahjones13 micahjones13 commented Jun 1, 2023

Brief Description

Adds dual-range capabilities to our rux-slider.
Refactors some of the markup and styles in order to be simpler and better match design.

  • Moved the datalist outside of the rux-slider div.
  • Refactored how opacity gets applied when disabled
  • Refactored the padding between slider and axis-labels to be 4px to match design

New Props

  • minVal
    • When present, adds a second thumb to the slider, creating a dual-range slider.
  • strict
    • In dual range mode, disables the ability for the thumbs to swap. ie, left thumb never passes right thumb and vice versa

JIRA Link

https://rocketcom.atlassian.net/browse/ASTRO-6206

Related Issue

#1073

General Notes

Currently, the bar between the thumbs in dual-range is not clickable/intractable. This is pending some more design research and could be changed in the future, but we left it out to prevent having to make a breaking change if the research says it shouldn't be clicked.

Motivation and Context

Adds dual-range variant to slider.
Dual-range has been an ask from clients.

Issues and Limitations

Dual range by default can have the thumbs swap over each other, and this can create instances where min-val is higher than value. A ticket has been made for this, but it is non-blocking for MVP.

Types of changes

  • Bug fix
  • New feature
  • Breaking change

Checklist

  • This PR adds or removes a Storybook story.
  • I have added tests to cover my changes.
  • Regressions are passing and/or failures are documented
  • Changes have been checked in evergreen browsers

@changeset-bot
Copy link

changeset-bot bot commented Jun 1, 2023

🦋 Changeset detected

Latest commit: c832fe6

The changes in this PR will be included in the next version bump.

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

@netlify
Copy link

netlify bot commented Jun 1, 2023

Deploy Preview for astro-stencil ready!

Name Link
🔨 Latest commit c832fe6
🔍 Latest deploy log https://app.netlify.com/sites/astro-stencil/deploys/64b5620a4d48fa000855ad9d
😎 Deploy Preview https://deploy-preview-1148--astro-stencil.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Jun 1, 2023

Deploy Preview for astro-preview ready!

Name Link
🔨 Latest commit c832fe6
🔍 Latest deploy log https://app.netlify.com/sites/astro-preview/deploys/64b5620a1ae2ae0007aee2c2
😎 Deploy Preview https://deploy-preview-1148--astro-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@micahjones13
Copy link
Collaborator Author

Looks great! Needs the test for the strict prop and for clicking right of the right thumb, then it looks good to go!

I can't for the life of me get the click to work in testing for moving the right thumb. Using the same method as I did to move the left thumb, but as soon as position y gets over 20 it breaks. I tried force: true but still no luck. If anyone has better ideas, plz let me know

Copy link
Contributor

@kiley-mitti kiley-mitti left a comment

Choose a reason for hiding this comment

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

Beautiful! Needs a changes but looks amazing. Great job Micah! It's brilliant. :)

@FMorrison87
Copy link
Contributor

I was testing programmatically updating the minVal and value prop and I noticed that the way the thumbs are set in the test file is with an attribute called max-val. Since this is not a prop when I update the component using the minVal and value props I get reflection on the min-val attribute, but the max-val attribute does not reflect the new value. Should it?

@micahjones13
Copy link
Collaborator Author

I was testing programmatically updating the minVal and value prop and I noticed that the way the thumbs are set in the test file is with an attribute called max-val. Since this is not a prop when I update the component using the minVal and value props I get reflection on the min-val attribute, but the max-val attribute does not reflect the new value. Should it?

ah great catch, that's a vestige of the first implementation. I'll fix that right now. Thank you!!

@micahjones13 micahjones13 merged commit c07d5f5 into main Jul 17, 2023
7 checks passed
@micahjones13 micahjones13 deleted the mj.dual-range-slider branch July 17, 2023 16:05
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.

4 participants