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

fix(NcActions): Show last action entry only partial to make it discoverable #5448

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Mar 29, 2024

☑️ Resolves

  1. Fix wrong event name - the event is named apply-show and not after-show
  2. If the popover content is too large and will be capped (max-height) make sure the last element is shown only partially, so users can discover there is more to scroll.

🖼️ Screenshots

See that before you can not see any hint of the overflow:

🏚️ Before 🏡 After
Screenshot_20240329_220236 Screenshot_20240329_220309

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

@susnux susnux added bug Something isn't working 3. to review Waiting for reviews feature: actions Related to the actions components labels Mar 29, 2024
@susnux susnux added this to the 8.11.2 milestone Mar 29, 2024
@susnux susnux force-pushed the fix/actions-show-last-partial branch from 9651b90 to 6974cf2 Compare March 29, 2024 21:12
@susnux
Copy link
Contributor Author

susnux commented Mar 29, 2024

cc @nextcloud-libraries/designers

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Screenshots look good to me :)
(but didnt test and didnt review the code)

Copy link
Contributor

@Pytal Pytal left a comment

Choose a reason for hiding this comment

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

Tested with different viewport heights and works nicely!

@Antreesy Antreesy mentioned this pull request Apr 9, 2024
…erable

1. Fix wrong event name - the event is named `apply-show` and not `after-show`
2. If the popover content is too large and will be capped (max-height) make sure the last element is shown only partially, so users can discover there is more to scroll.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the fix/actions-show-last-partial branch from 6974cf2 to 04811b1 Compare April 9, 2024 20:24
@susnux susnux merged commit 11263a1 into master Apr 9, 2024
18 checks passed
@susnux susnux deleted the fix/actions-show-last-partial branch April 9, 2024 20:30
@susnux
Copy link
Contributor Author

susnux commented Apr 9, 2024

/backport to next

@jancborchardt
Copy link
Contributor

@susnux we also need to keep in mind this one though: #3920
Yes long menus should have the last entry halved, but in general the menus can take up more vertical space.

@susnux
Copy link
Contributor Author

susnux commented Apr 11, 2024

but in general the menus can take up more vertical space.

Yes but that is a different story and should be handled as its own. This only fixes menus overflowing our "maximum" to show the last one half.

@susnux
Copy link
Contributor Author

susnux commented May 30, 2024

Actually not sure, we do change the event name by catching it in the NcPopover

We also forward all listeners so we should be good either way. But seems you are right...
A bit weird to have two events with exactly the same meaning :/

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 bug Something isn't working feature: actions Related to the actions components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Specific screen-resolutions can hide options in the new-file (of type) pulldown
5 participants