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

Dots Partially Hidden with parent element's overflow set to hidden #337

Closed
ericdrobinson opened this issue Apr 2, 2019 · 4 comments
Closed
Labels

Comments

@ericdrobinson
Copy link

ericdrobinson commented Apr 2, 2019

Describe the bug

Dots get partially cut off when the slider's parent element has overflow: hidden; set.

Additional context

The current padding calculation appears to differentiate between horizontal and vertical slider modes, applying padding in only the cross-axis direction (presumably for "stacking"). However, this does not take into account the fact that the dots will extend beyond the bounds of the container in the "lengthwise-axis" of the rail unless extra padding is applied.

This JSFiddle shows the issue:
image

In the above example, the first slider uses the current default padding settings. The second slider swaps the "horizontal" and "vertical" padding axes (which is obviously wrong). The third slider is set to have identical horizontal and vertical padding (as the dot is a circle).

I believe that this is the offending line of code. A simple fix would be to replace the above with the following logic:

padding: `${dotHeight / 2}px ${dotWidth / 2}px`,

Environment

  • Vue version: v2.6.10
  • Component Version: 3.0.22
@NightCatSama
Copy link
Owner

In fact, it used to be what you said.

Because if the component uses horizontal padding, it will cause the head and tail of the component to be out of alignment with other elements. #319

You can add padding to the container, for example: https://jsfiddle.net/Lphwac7q/

@ericdrobinson
Copy link
Author

In fact, it used to be what you said.

Yup. I noticed the issue when we updated to 3.x.

Because if the component uses horizontal padding, it will cause the head and tail of the component to be out of alignment with other elements. #319

Sure. I wouldn't say that it's "out of alignment with other elements", but rather that the "dots" may not appear to be "centered" with respect to the edges of other elements. This is a design choice. In our case, it appears wrong.

You can add padding to the container, for example: https://jsfiddle.net/Lphwac7q/

This requires adding a container element for the slider itself. Would it not be possible to update the padding logic to support both scenarios? Perhaps by adding a boolean contained prop that specifies whether or not the slider should be fully contained within its containing element?

Theoretically, support for this would look like:

padding: this.contained ? `${dotHeight / 2}px ${dotWidth / 2}px` : (this.isHorizontal ? `${dotHeight / 2}px 0` : `0 ${dotWidth / 2}px`),

This would allow for the following:

  1. Explicit documentation of the way that the dots may extend outside of the bounds of the containing element (default).
  2. A clean and simple way to have the slider draw correctly without adding another element to the DOM hierarchy.

@NightCatSama
Copy link
Owner

Added the contained parameter in version 3.0.23.

https://nightcatsama.github.io/vue-slider-component/#/basics/simple?hash=contained

Thank you for your feedback.

@ericdrobinson
Copy link
Author

Well, damn. That was fast! Thanks very much for handling this! The documentation example is extremely clear, by the way. Awesome! Hopefully it will help others easily implement their designs!

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

No branches or pull requests

2 participants