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

chore(widgets): remove collapse menu item #10449

Merged
merged 1 commit into from Apr 11, 2017
Merged

Conversation

mrclay
Copy link
Member

@mrclay mrclay commented Oct 15, 2016

(this was broken out of #10405)

BREAKING CHANGE
The collapse widget menu item has been removed.

@hypeJunction
Copy link
Contributor

We don't have break as allowed commit type...

@mrclay mrclay changed the title break(widgets): remove collapse menu item chore(widgets): remove collapse menu item Oct 15, 2016
@jdalsem
Copy link
Member

jdalsem commented Oct 17, 2016

why? for what reason should this be removed?

@hypeJunction
Copy link
Contributor

For what reason does it exist? To me it makes no sense. If we were to preserve the toggle state as a user preference, sure, let's keep it. Otherwise, I don't see a reason to keep it around.

@jdalsem
Copy link
Member

jdalsem commented Oct 17, 2016

If we were to preserve the toggle state as a user preference, sure, let's keep it.

that is what Widget Manager plugin did... makes it indeed so much more useful... so i suggest we move that feature to core and make it useful instead of removing it

@hypeJunction
Copy link
Contributor

@hypeJunction
Copy link
Contributor

I don't want it in the menu. Widget manager can just re-add the menu item. It makes little sense in core.

@jdalsem
Copy link
Member

jdalsem commented Oct 17, 2016

It makes little sense in core.

it does if the feature works as intended

@hypeJunction
Copy link
Contributor

it does if the feature works as intended

It doesn't, so unless you make it work as intended, I don't see a point in keeping it. All the half-baked API in core needs to get lost.

@jdalsem
Copy link
Member

jdalsem commented Oct 17, 2016

It doesn't, so unless you make it work as intended, I don't see a point in keeping it.

I'll give it a shot

All the half-baked API in core needs to get lost.

Totally agree

@hypeJunction
Copy link
Contributor

In terms of UX, it makes sense to store the preference for the viewer and not the owner of the widget, which makes it quite data intensive.

@jdalsem
Copy link
Member

jdalsem commented Oct 17, 2016

In terms of UX, it makes sense to store the preference for the viewer

that is what we do...

which makes it quite data intensive

only if you toggle... (same intensity as a like)

@mrclay
Copy link
Member Author

mrclay commented Apr 10, 2017

I'll give it a shot

@jdalsem Y'all had 6 months to get this in. I'm going to clean this up and get it in. You can always revert this in your work.

Widget menu items now are floated instead of absolutely positioned.

BREAKING CHANGE
The collapse widget menu item has been removed.
@mrclay
Copy link
Member Author

mrclay commented Apr 10, 2017

It's ready again.

@mrclay mrclay added this to the Elgg 3.0.x milestone Apr 10, 2017
@jdalsem jdalsem merged commit 79ec0ca into Elgg:master Apr 11, 2017
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