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

No way to remove a usersettings view #9704

Closed
hypeJunction opened this Issue Apr 15, 2016 · 11 comments

Comments

Projects
None yet
4 participants
@hypeJunction
Contributor

hypeJunction commented Apr 15, 2016

I want to get rid of user settings for a certain plugin. If I overwrite the view with an empty string, it still renders the form without the body.
I quite dislike our user settings magic. We need to reconsider our approach.

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Apr 16, 2016

Contributor

I propose we stop outputting empty form markup from elgg_view_form (). Can we consider it a bugfix, or would somebody be relying on having empty forms without any fields and buttons?

Contributor

hypeJunction commented Apr 16, 2016

I propose we stop outputting empty form markup from elgg_view_form (). Can we consider it a bugfix, or would somebody be relying on having empty forms without any fields and buttons?

@beck24

This comment has been minimized.

Show comment
Hide comment
@beck24

beck24 Apr 16, 2016

Member

I can't imagine a scenario where someone would rely on that. This has annoyed me also.

Member

beck24 commented Apr 16, 2016

I can't imagine a scenario where someone would rely on that. This has annoyed me also.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Apr 20, 2016

Member

Better to fix this in 2.x by adding an option to the input/form view.

$allow_empty_body = elgg_extract('allow_empty_body', $vars, true);
Member

mrclay commented Apr 20, 2016

Better to fix this in 2.x by adding an option to the input/form view.

$allow_empty_body = elgg_extract('allow_empty_body', $vars, true);

@mrclay mrclay reopened this Apr 20, 2016

@jdalsem

This comment has been minimized.

Show comment
Hide comment
@jdalsem

jdalsem Apr 20, 2016

Member

shouldn't it be default false to be BC?

Member

jdalsem commented Apr 20, 2016

shouldn't it be default false to be BC?

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Apr 20, 2016

Member

Maybe "allow_empty_body" would be clearer? This is the default behavior, so default true.

If false, then we return if $body is empty.

Member

mrclay commented Apr 20, 2016

Maybe "allow_empty_body" would be clearer? This is the default behavior, so default true.

If false, then we return if $body is empty.

@jdalsem

This comment has been minimized.

Show comment
Hide comment
@jdalsem

jdalsem Apr 20, 2016

Member

Maybe "allow_empty_body" would be clearer

clearer indeed

This however makes it a feature and it means we can not fix it (having empty forms) in core... until 3.x

Member

jdalsem commented Apr 20, 2016

Maybe "allow_empty_body" would be clearer

clearer indeed

This however makes it a feature and it means we can not fix it (having empty forms) in core... until 3.x

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Apr 20, 2016

Member

Having it default to current behavior allows adding in 2.x.

Member

mrclay commented Apr 20, 2016

Having it default to current behavior allows adding in 2.x.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Apr 20, 2016

Member

The fix would also involve:

  • change view resources/settings/tools to set the option to false.
  • change view form/plugins/settings/save to return if no settings. I think that's acceptable for 2.x
Member

mrclay commented Apr 20, 2016

The fix would also involve:

  • change view resources/settings/tools to set the option to false.
  • change view form/plugins/settings/save to return if no settings. I think that's acceptable for 2.x

@mrclay mrclay closed this in 5f7dbcd Apr 21, 2016

@hypeJunction hypeJunction reopened this Apr 21, 2016

@jdalsem

This comment has been minimized.

Show comment
Hide comment
@jdalsem

jdalsem Sep 27, 2016

Member

@hypeJunction why was this reopened?? i lost track

Member

jdalsem commented Sep 27, 2016

@hypeJunction why was this reopened?? i lost track

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Sep 27, 2016

Contributor

I have no idea. I think we reverted the changes in core

Contributor

hypeJunction commented Sep 27, 2016

I have no idea. I think we reverted the changes in core

@jdalsem

This comment has been minimized.

Show comment
Hide comment
@jdalsem

jdalsem Jul 25, 2017

Member

Should be fixed by #11011

Member

jdalsem commented Jul 25, 2017

Should be fixed by #11011

@jdalsem jdalsem closed this Jul 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment