Skip to content

Conversation

@szaimen
Copy link
Contributor

@szaimen szaimen commented Nov 8, 2022

Fix nextcloud/server#33839

Before After
image image

Signed-off-by: Simon L [email protected]

@szaimen szaimen added bug Something isn't working 3. to review Waiting for reviews labels Nov 8, 2022
@szaimen szaimen added this to the 7.1.0 milestone Nov 8, 2022
@szaimen szaimen requested review from a team, PVince81, Pytal, juliusknorr and skjnldsv and removed request for a team November 8, 2022 11:47
@szaimen
Copy link
Contributor Author

szaimen commented Nov 8, 2022

/backport to stable7

@skjnldsv
Copy link
Contributor

skjnldsv commented Nov 8, 2022

Is there no way for the popper element to automatically detect out-of-bound window ?

Also, if you add 10 links and open the last menu, isn't the issue going to be the same?

@szaimen
Copy link
Contributor Author

szaimen commented Nov 8, 2022

Is there no way for the popper element to automatically detect out-of-bound window ?

Honestly, I don't know.

Also, if you add 10 links and open the last menu, isn't the issue going to be the same?

I can test this case but I think it should work correctly.

@szaimen
Copy link
Contributor Author

szaimen commented Nov 8, 2022

Also, if you add 10 links and open the last menu, isn't the issue going to be the same?

I can test this case but I think it should work correctly.

@skjnldsv shall I test it?

@juliusknorr
Copy link
Contributor

I think the 50vh approach might also not work well if the header is larger (e.g. with a preview). Maybe we can change the popper positioning strategy so that the popover is not limited to the tab area but may also span over the full window height and be positioned to the left of the button?

@juliusknorr
Copy link
Contributor

Is there no way for the popper element to automatically detect out-of-bound window ?

Might not work in a generic way, so maybe that is something that needs rather some fine tuning in the sharing sidebar implementation.

@szaimen
Copy link
Contributor Author

szaimen commented Nov 9, 2022

I think the 50vh approach might also not work well if the header is larger (e.g. with a preview)

No, that is no problem as it will overlap the preview header then. Let me test this out as well...

@szaimen
Copy link
Contributor Author

szaimen commented Nov 9, 2022

@skjnldsv @juliushaertl both edge cases seem to work:
image

@szaimen
Copy link
Contributor Author

szaimen commented Nov 9, 2022

@skjnldsv @juliushaertl can you approve? :)

@szaimen
Copy link
Contributor Author

szaimen commented Nov 9, 2022

One additional review needed :)

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 nice

@skjnldsv skjnldsv mentioned this pull request Nov 15, 2022
@nickvergessen
Copy link
Contributor

/backport to stable7

@backportbot-nextcloud
Copy link

The backport to stable7 failed. Please do this backport manually.

@szaimen
Copy link
Contributor Author

szaimen commented Nov 17, 2022

/backport to stable7

@backportbot-nextcloud
Copy link

The backport to stable7 failed. Please do this backport manually.

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 backport-request bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

some elements in the sharing dropdown are not visible and no way to scroll down in certain screen resolutions

7 participants