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

WIP Entity menu ellipsis #10419

Closed
wants to merge 2 commits into from
Closed

WIP Entity menu ellipsis #10419

wants to merge 2 commits into from

Conversation

hypeJunction
Copy link
Contributor

entity

Popup modules can now be popped preserving their DOM position by
adding .elgg-popup-inline to the trigger element classes
BREAKING CHANGE
All entity menu items registered with non-default viewtype will be
displayed in a popup menu with an ellipsis trigger
@hypeJunction
Copy link
Contributor Author

We can do this for objects right away. With users, we can decide whether we keep the user hover menu or not. I'd rather we get rid of it and start consolidating around entity menu for all actions.

'text' => elgg_view_icon('ellipsis-v'),
'href' => "#{$id}",
'class' => 'elgg-popup-inline',
'data-position' => json_encode([
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should go in a JS hook.

@jdalsem
Copy link
Member

jdalsem commented Oct 11, 2016

are these menus ajax loaded? or is that now gone...

@hypeJunction
Copy link
Contributor Author

Now ajax

@hypeJunction
Copy link
Contributor Author

No more ajax. Can be added I suppose but that doesn't make sense for all entity menus

@jdalsem
Copy link
Member

jdalsem commented Oct 11, 2016

when we moved the userhover menu to an ajax hook this made a significant improvement ... would not like to see us go back a few years.. why would it not apply to all entity menus?

@hypeJunction
Copy link
Contributor Author

Because they only have a few items. We can make it ajax but it's a whole new level of complexity. This is just a demo. We don't have to move uset hover yet.

@jdalsem
Copy link
Member

jdalsem commented Oct 11, 2016

Because they only have a few items.

maybe for now...

We can make it ajax but it's a whole new level of complexity. This is just a demo. We don't have to move uset hover yet.

it makes sense to do a similar ellipsis for user hover

potential downside of hidden menus is the fact that the will never be used, but still require server processing. Show 50 entities with 5 menu items with all the hooks... that will take some time. It was for users anyway... entities will be a lot less, but the ellipsis menu will encourage usage of the entity menu, so we should prepare for that

@jdalsem
Copy link
Member

jdalsem commented Oct 11, 2016

i like where this is going btw...

does this mean all entity menu items are in the ellipsis? i was wondering if we could separate primary (always visible) and secondary (in the ellipsis) menu items

You now set different colors for various sections:
i was wondering if we could create some default styled/colored sections for

  • generic actions
    • send message
    • add friend
  • owner actions
    • edit content
  • group admin actions
    • kick from group
    • make group admin
  • admin actions
    • delete user
    • ban

@jdalsem
Copy link
Member

jdalsem commented Oct 11, 2016

does this mean all entity menu items are in the ellipsis? i was wondering if we could separate primary (always visible) and secondary (in the ellipsis) menu items

is see you created a discussion for that here: #10420

@jdalsem
Copy link
Member

jdalsem commented Oct 11, 2016

maybe you could also take a look at #6936 while you are at it

@hypeJunction
Copy link
Contributor Author

There is also this bugger on the way #9126.

@jdalsem
Copy link
Member

jdalsem commented Oct 11, 2016

there are always items that serve both an informational purpose and also have an action, like the like/comment actions + counts or the access information (combined with plugins like this https://github.com/hypeJunction/Elgg-menus_access)... those menu items should not be part of the ellipsis. I would opt to make entity menu items by default be part of the ellipsis section and if you need it elsewhere, you register for an other section, that will just show inline (like it does now)

@hypeJunction
Copy link
Contributor Author

We can't really make this work with ajax without breaking the menu down into different menus (no way of telling when the hook should be adding items). Perhaps we could keep user hover as a menu loaded via ajax but move the trigger to the entity menu. And add similar menus for objects and groups. I am letting this site for a bit until I there is some clear roadmap

@jeabakker
Copy link
Member

if you come back to this, maybe also look at #6402

@hypeJunction
Copy link
Contributor Author

I have a different approach in mind. Will resubmit in another PR

@hypeJunction
Copy link
Contributor Author

Strawman:

  • Teach elgg/popup to load ajax views as we do in tabs
  • Add more menu item that links to the appropriate view with the ellipsis menu (rename user_hover to entity:more)
  • Render entity menu regardless of context, inject all view vars to entity menu, let plugins decide when to render menu items

@hypeJunction
Copy link
Contributor Author

Another PR will be coming eventually.

@hypeJunction hypeJunction deleted the ellipsis branch March 31, 2017 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants