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.

Sign up for GitHub

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

Jump to bottom

add a "none" option for the top term #1082

Closed
aschweigert opened this issue Jan 20, 2016 · 2 comments
Closed

add a "none" option for the top term #1082

aschweigert opened this issue Jan 20, 2016 · 2 comments
Assignees
@benlk
Labels
priority: high Either blocks work on a priority-normal task or a solution here informs other work. type: improvement
Milestone
0.5.5 - Story Ele...

Comments

@aschweigert
Copy link

currently we don't give people the option to not set a top term (even if it's something lame like "uncategorized"). We should add a "none" option to the dropdown menu in the admin and account for this in the front end display of the top tag (by just not returning any of the markup).

needed for RNS-127
@aschweigert aschweigert added type: improvement priority: high Either blocks work on a priority-normal task or a solution here informs other work. labels Jan 20, 2016
@aschweigert aschweigert self-assigned this Jan 20, 2016
@aschweigert aschweigert added this to the 0.5.5 - Story Elements milestone Jan 20, 2016
@aschweigert aschweigert assigned benlk and unassigned aschweigert Jan 21, 2016
@benlk
Copy link
Collaborator

benlk commented Jan 21, 2016

This is going to involve changes in:

  • largo_post_social_links to support a 'none' option for the top term, else it will try to get the taxonomy of the empty term.
  • largo_top_tag_display to include a 'none' option
  • the validation function saved with largo_register_meta_input( 'top_term', 'intval' ); that are called at https://github.com/INN/Largo/blob/f4ddf0b27b5fc34fb826e7ac22a9c393b4cbed1a/inc/metabox-api.php#L135: this needs to not be intval, because the term id cannot be '0' because if it's '0' or 0 or '' or any other value that computes as false-y, then largo [deletes the saved option]: therefore this has to be a string.

benlk added a commit that referenced this issue Jan 21, 2016
@benlk
Support a "None" option for the top term, for #1082
a95593a 
- adds 'none' option to largo_top_tag_display metabox function
- changes the validation function for the top term from `intval` to `sanitize_key` because "None" cannot have an id of `'0'` because `'0'` and `0` and `''` are falsy and therefore _largo_meta_box_post_save will remove the option instead of saving it
- change largo_top_term so that it outputs `''` if the top term option is `'null'`
This was referenced Jan 21, 2016
Merged
Closed
@benlk
Copy link
Collaborator

benlk commented Jan 28, 2016

Closing this because the none option is available as of #1090 and #1092 has been created.

@benlk benlk closed this as completed Jan 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@benlk benlk

Labels
priority: high Either blocks work on a priority-normal task or a solution here informs other work. type: improvement
Projects
None yet
Milestone
0.5.5 - Story Elements
Development

No branches or pull requests
2 participants
@aschweigert @benlk