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

Share multiselect overlapped on small screens #23617

Closed
PVince81 opened this issue Oct 22, 2020 · 25 comments · Fixed by #25503
Closed

Share multiselect overlapped on small screens #23617

PVince81 opened this issue Oct 22, 2020 · 25 comments · Fixed by #25503
Assignees
Labels
1. to develop Accepted and waiting to be taken care of bug regression
Milestone

Comments

@PVince81
Copy link
Member

Steps

  1. Upload a picture
  2. Open the details panel
  3. Open the sharing tab
  4. Reduce the window size so that the sharing tab has a scrollbar and uses less than 30% of the screen
  5. Type some text in the field
  6. Try to select something in the multiselect dropdown

Expected result

Can select something.
And if not, at least a way to expand that panel or reduce the preview size to be able to see the panel better.

Actual result

Multiselect dropdown is appearing behind the tabs so cannot see anything
image

I tried to quickly set z-index on the ".multiselect" element but that didn't help. Might need more advanced CSS engineering or even shifting DOM nodes.

Versions

Observed on git master 872f032

In Nextcloud Talk 20 the dropdown appears correctly above, so this is a regression!

@PVince81 PVince81 added bug 1. to develop Accepted and waiting to be taken care of regression labels Oct 22, 2020
@PVince81 PVince81 added this to the Nextcloud 21 milestone Oct 22, 2020
@PVince81
Copy link
Member Author

PVince81 commented Oct 22, 2020

  • check stable20 => looks fine there
  • bisect regression

@PVince81
Copy link
Member Author

PVince81 commented Oct 22, 2020

I've bisected it but the result doesn't make sense. It points at a commit that doesn't contain any code related to the frontend: 6c1b542

And yet, when I jump back on the previous commit, the autocomplete appears correctly.
And when I jump to that commit, the bug occurs.

I tried that repeatedly with various levels of cleanup intensity, and it still happens.

Really weird...

@PVince81
Copy link
Member Author

PVince81 commented Oct 22, 2020

What?!

If I check out 6c1b542 and revert the version.php from 21.0.0.1 to 21.0.0.0, the dropdown works again ?! (with full cleanup)

@PVince81
Copy link
Member Author

Trying again without extra apps, just in case, and with production build

@PVince81
Copy link
Member Author

Aha, after resetting the apps folder to what git provides, the error is gone.

So it means I had an app in there which CSS caused side effects. Hmmm... And maybe it got auto-enabled on 21.0.0.1 but not 21.0.0.0... Investigating...

@PVince81 PVince81 self-assigned this Oct 22, 2020
@PVince81
Copy link
Member Author

I had the photos app version 1.3.0 (git 63897ad6eb25cce805fe419eac161b97f9db21e9) in my apps folder.

For both version.php 21.0.0.0 or 21.0.0.1 the app gets auto-enabled, so it's not an auto-enable bug.

Now to find out why a CSS from an app would be different based on version.php...

@PVince81
Copy link
Member Author

I updated the git code from the photos app, now sitting on e4b9123c7488fb2d1d17889f5e0f93d120038fa3
Now: if I only build from the server repo with make dev-setup && make build-js, I heard that it would also compile the scss from apps. => here the problem occurs.
However if I run make dev-setup && make build-js in the app's folder, then the problem disappears.

As an app dev I'd expect doing both of these operations to have the same effect ?
In general as an app dev I wouldn't expect the server repo to compile my app's assets.

@rullzer @skjnldsv

@PVince81
Copy link
Member Author

closing as the css problem is gone, but might want a new issue to discuss the compiling bits

@skjnldsv
Copy link
Member

I heard that it would also compile the scss from apps. => here the problem occurs.

The only scss we have is https://github.com/nextcloud/photos/blob/master/css/icons.scss
Another thing is managed by webpack directly on photos. No interferences with the webpack build on server

@PVince81
Copy link
Member Author

it's back:
image

and this time I have no photos app.

and I have the original code from master, no files compiled nor changed.

reopening to investigate further

@PVince81 PVince81 reopened this Nov 18, 2020
@skjnldsv
Copy link
Member

Shoudld be fixed with the open-direction="below" prop https://vue-multiselect.js.org/#sub-props

@PVince81
Copy link
Member Author

I can try. However when a preview image is present, there might not be enough room to display it downwards.

In general I guess we should perhaps reduce the preview size on smaller screens (small dpi-wise, I have 1080p but with higher dpi so everything looks bigger)

@PVince81
Copy link
Member Author

If I remove the overflow of the tab the dropdown appears again, but it messes up the scrollbars:
image

@skjnldsv
Copy link
Member

Of course :p
I mean, we don't do any height-responsiveness on Nextcloud at the moment, only width is being handled for mobile view.

I guess it's fine if we just set it to bottom for now

@PVince81
Copy link
Member Author

PVince81 commented Nov 18, 2020

okay, I had your branch checked out: nextcloud-libraries/nextcloud-vue#1579

and locally added open direction and it looks better, but there's another unrelated issue which I guess you are working on already:
image

I like that the multiselect adjusts to the remaining height.

Next steps:

@PVince81
Copy link
Member Author

The error disappeared when I switched to nextcloud-vue master.

@blizzz
Copy link
Member

blizzz commented Nov 25, 2020

yesterday i've had it with a full screen window with master.

@PVince81
Copy link
Member Author

PVince81 commented Feb 5, 2021

still visible on stable21 5ec0132 (with no extra files)

image

@PVince81
Copy link
Member Author

PVince81 commented Feb 5, 2021

also happening with the 21.0.0RC1 release tarball

@PVince81
Copy link
Member Author

PVince81 commented Feb 5, 2021

I've tried #24694 (rebased onto master + recompiled), but it doesn't fix this issue.
So maybe it's not a boundary thing.

@PVince81
Copy link
Member Author

PVince81 commented Feb 5, 2021

a short in the dark: I tried updating nextcloud-vue to 3.5.4 but it also doesn't fix the issue either.

@PVince81
Copy link
Member Author

PVince81 commented Feb 5, 2021

I had a look deeper into vue-multiselect and it appears that the dropdown is always appended within the same container, not in the body. There is no option to make it use a different containment.

@PVince81
Copy link
Member Author

PVince81 commented Feb 5, 2021

Seems this is still on a TODO list for the next major vue-multiselect version: shentao/vue-multiselect#618

I see some possible workarounds in that ticket

@PVince81
Copy link
Member Author

PVince81 commented Feb 5, 2021

okay, back to the old solution: setting the direction to "below".

I'll push a PR

@PVince81
Copy link
Member Author

PVince81 commented Feb 5, 2021

PR here: #25503

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants