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

NcSelect: Selected options should not overflow the container #3759

Merged
merged 1 commit into from Feb 22, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Feb 15, 2023

If the selected option is longer than the element it should not overflow the NcSelect but use ellipsis.

before with min-width: 0 with hidden disabled search
before after after-2

As you can see currently the option overflows the select, note the actions (dropdown toggle) are missing because they are placed outside the select underneath other elements on the right.

After fixing the overflow there is still this weird height issues, which happens because .vs__selected-options have flex-wrap set and contain the search input even if it is disabled (searchable=false).
This can be fixed by removing the search input from the visible dom when not used.

I already submitted a upstream PR, but I doubt that it will be merged soon.

@susnux susnux added 3. to review Waiting for reviews design Design, UX, interface and interaction design feature: select Related to the NcSelect* components labels Feb 15, 2023
@susnux susnux requested a review from a team February 15, 2023 18:09
@susnux susnux requested review from Pytal and artonge February 15, 2023 19:02
@raimund-schluessler

This comment was marked as resolved.

raimund-schluessler

This comment was marked as resolved.

Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
@susnux
Copy link
Contributor Author

susnux commented Feb 15, 2023

Fixing the width issue is fine, but the selected option still overflows 🖼️

This is caused by position: absolute added when opening the menu.
I fixed this by setting a max-width.

But, hiding the input field prevents the dropdown from closing. It now only closes when you select an option, but not when you click outside anymore.

Ah did not noticed that, thank you! Fixed that as well.

@susnux
Copy link
Contributor Author

susnux commented Feb 15, 2023

PR for the nextcloud fork of the vue-select: nextcloud-deps/vue-select#18

(Forgot that we are using a custom fork and not the vanilla upstream)

@mejo- mejo- merged commit 9dd000a into master Feb 22, 2023
@mejo- mejo- deleted the fix/nc-select-small branch February 22, 2023 11:26
Comment on lines +1025 to +1030
.vs__selected-options {
min-width: 0;
.vs__selected {
min-width: 0;
}
}
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Design, UX, interface and interaction design feature: select Related to the NcSelect* components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants