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

Mismatching z-index in Multiselect.vue and vue-multiselect.min.css #850

Closed
muzhig opened this issue Oct 12, 2018 · 9 comments
Closed

Mismatching z-index in Multiselect.vue and vue-multiselect.min.css #850

muzhig opened this issue Oct 12, 2018 · 9 comments

Comments

@muzhig
Copy link

muzhig commented Oct 12, 2018

Reproduction Link

z-index: 50 in vue sourcefile:
https://github.com/shentao/vue-multiselect/blob/master/src/Multiselect.vue#L639

z-index: 1 in minimized css:
https://github.com/shentao/vue-multiselect/blob/master/dist/vue-multiselect.min.css

Visualization
https://jsfiddle.net/jqofkzxc/6422/

Steps to reproduce

Open dropdown

Expected behaviour

red label is under the dropdown

Actual behaviour

red label is above the dropdown

@shentao
Copy link
Owner

shentao commented Oct 12, 2018

This is actually a pretty decent behaviour. You can control whether the element should be above something else or not. And it can be easily changed like here: https://jsfiddle.net/shentao/r912znhf/.

Also, the CSS class you pointed at, is related to the dropdown list and not the component itself.

@JBtje
Copy link

JBtje commented Oct 17, 2018

I don't really get the label "invalid".
The problem is that the production values differ from the dist value, that is a fact.

Please have a look at this:
https://jsfiddle.net/JBtje/r912znhf/2/

Yes, it can be solved by doing some manual css overwriting, but the problem is still there in the dist version... I say: bug.

@muzhig
Copy link
Author

muzhig commented Oct 23, 2018

Indeed, I probably named my issue wrong. My main concern was inconsistency between production and development code.

@shentao
Copy link
Owner

shentao commented Oct 23, 2018

@jbruni It does not differ. Pay more attention to the class names.

@JBtje
Copy link

JBtje commented Oct 24, 2018

@shentao could you elaborate?

https://github.com/shentao/vue-multiselect/blob/master/src/Multiselect.vue#L446-L448

.multiselect--active {
  z-index: 50;
}

https://github.com/shentao/vue-multiselect/blob/master/dist/vue-multiselect.min.css#L1
characters: 1224:1255

.multiselect--active{z-index:1}

https://github.com/shentao/vue-multiselect/blob/master/src/Multiselect.vue#L628-L641

.multiselect__content-wrapper {
  position: absolute;
  display: block;
  background: #fff;
  width: 100%;
  max-height: 240px;
  overflow: auto;
  border: 1px solid #e8e8e8;
  border-top: none;
  border-bottom-left-radius: 5px;
  border-bottom-right-radius: 5px;
  z-index: 50;
  -webkit-overflow-scrolling: touch;
}

https://github.com/shentao/vue-multiselect/blob/master/dist/vue-multiselect.min.css#L1
characters: 4156:4421

.multiselect__content-wrapper{position:absolute;display:block;background:#fff;width:100%;max-height:240px;overflow:auto;border:1px solid #e8e8e8;border-top:none;border-bottom-left-radius:5px;border-bottom-right-radius:5px;z-index:1;-webkit-overflow-scrolling:touch}

I don't see a difference in class name, I do however see a difference in production and master branch code. But perhaps i'm missing something?

@shentao shentao removed the invalid label Oct 25, 2018
@shentao
Copy link
Owner

shentao commented Oct 25, 2018

Oh noes. So sorry about my previous replies – turns out I was the one to misread the code... I really should have paid more attention to it.

I guess I wrongly assumed that the build step wouldn’t touch the z-index values. Turns out it does in a previous version of optimize-css-assets-webpack-plugin (and the cssnano version it uses) used here. Couldn’t update to a newer version, since it requires a newer version of webpack. However, I updated cssnano and now the output should be aligned with the default config, leaving z-index untouched.
Will be doing a release in a few next days.

Sorry for the trouble. Hope you understand @muzhig @JBtje.

@JBtje
Copy link

JBtje commented Oct 26, 2018

Much appreciated! (and glad i'm nog losing my mind :))

@chenxeed
Copy link

Thanks for the fixes @shentao ! <3

Can the fixes be released soon? I prefer to fetch the latest code from the new tag like 2.1.4 rather than from master directly.

Thank you!

@surajitbasak109
Copy link

This is actually a pretty decent behaviour. You can control whether the element should be above something else or not. And it can be easily changed like here: https://jsfiddle.net/shentao/r912znhf/.

Also, the CSS class you pointed at, is related to the dropdown list and not the component itself.

Thanks it worked!! 👍

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

No branches or pull requests

5 participants