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

Feature/noid/avatar menu use actions component #1670

Closed

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Jan 19, 2021

This makes the Avatar component use the Actions component for its menu.

image

Issues:

  • need a way to set button size for Actions from attributes
  • opening the menu has the repositioning/resize glitch from Use the popover component for the avatar menu #831, will need a different approach for differentiating "open state" vs "data loaded and safe to display" state. Maybe need to add a loading attribute to the actions
  • keyboard nav still not working, can't enter the menu. Seems to be a bug in Actions component.
  • make sure that all action types are supported, need to check all the formats returned by the endpoint

marcoambrosini and others added 8 commits January 18, 2021 11:46
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
Use the correct variable for iterating over the menu items

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Fix avatar popover trigger placement and alignment.
Fix icon size in menu item to be 16px instead of 44px.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
VPopover is always visible even when "open" is false.
This fix makes it hidden visually off screen instead.

Solves issues with flying arrows when opening and closing some action
menus.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Whenever the popover is set to "open" first, if the contents hasn't
already been populated in the current tick, the popover's position will
appear in the wrong location. Then, directly in the next tick its
position is corrected based on the newly appeared content.

To avoid the visual glitch we first keep it with "visibility: hidden"
upon opening, and then display it once its position has been calculated.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
When initially hiding the popover during size / position calculation, we
need to override the popover's initial visibility and position to avoid
glitches like scrollbars appearing.

Once the correct position has been calculated by v-tooltip/popper, we
can remove the strong hiding class.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@nickvergessen
Copy link
Contributor

I think this is the wrong way. Instead one of the next todos is to blow the menu up and show a bigger avatar similar to what github does:
Bildschirmfoto von 2021-01-20 12-14-03

@PVince81
Copy link
Contributor Author

Right, and we still need a place for the actions. Maybe a sub-menu inside that dialog, which itself will be the Actions menu.
The fact that actions are server-defined makes it more tricky.

Was there an existing discussion somewhere already about this new design ?

@raimund-schluessler
Copy link
Contributor

What is the plan here? The Avatar component is the last one to use the deprecated PopoverMenu component and I think it would be good to migrate away from it.

@PVince81 PVince81 assigned Pytal and unassigned PVince81 Jan 30, 2023
@raimund-schluessler
Copy link
Contributor

Superseded by #4017.

@raimund-schluessler raimund-schluessler deleted the feature/noid/avatar-menu-use-actions-component branch April 29, 2023 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove PopoverMenu and PopoverMenuItem components
5 participants