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

NcSelect: Selected options should not overflow the container #3759

Merged
merged 1 commit into from
Feb 22, 2023
Merged

NcSelect: Selected options should not overflow the container #3759

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions src/components/NcSelect/NcSelect.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,36 @@ body {
}
}
}

// Hide search from dom if unused to prevent unneeded flex wrap
.vs__search[readonly] {
position: absolute;
}
// If search if hidden, ensure that the height of the search is the same
.vs__selected-options {
min-height: 40px; // 36px search height + 4px search margin
}

/**
* Fix overlow of selected options
* There is an upstream pull request, if it is merged and released remove this fix
* https://github.com/sagalbot/vue-select/pull/1756
*/
.vs__selected-options {
min-width: 0;
.vs__selected {
min-width: 0;
}
}
Comment on lines +1025 to +1030
Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed that these two lines create issues when the NcSelect component has :no-wrap="true"set. See here:
Screenshot 2023-02-22 at 12-45-19 Nextcloud Vue Style Guide

Copy link
Contributor

Choose a reason for hiding this comment

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

@susnux I created a new issue for this: #3808

Copy link
Contributor

Choose a reason for hiding this comment

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

Mh, shall we revert the PR for this release @raimund-schluessler?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really have a preference here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there's a need for rush, so let's try to get this solved before releasing 7.7.0. @susnux would you prefer to look for a fix or to revert the PR for 7.7.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will look for a fix.

&.vs--single {
&.vs--loading,
&.vs--open {
.vs__selected {
// Fix `max-width` for `position: absolute`
max-width: 100%;
}
}
}
}

.vs__dropdown-menu {
Expand Down