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

Refactor the saved links display metabox #80

Closed
wants to merge 16 commits into from

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Aug 13, 2015

Changes

  • cloned Wordpress' WP_List_Table class file to inc/saved-links and renamed its class to clone_WP_List_Table. The reason for cloning this and not extending it is because WP_List_Table is a private, unstable class that developerssay should not be directly extended.
  • Extended clone_WP_List_Table to create Saved_Links_List_Table with rendering options, columns, and a query specific for the lroundups-links posts.
  • Changed the saved links display page to an instance of Saved_Links_List_Table
  • Changed the "Send to editor" button and other other JavaScripts to reflect changed selectors.
  • Added a spinner to the "Filter" button to indicate that the links are loading.
  • CSS for miscellaneous pieces, for uniform looks
    • thead td and thead td checkbox so they appear the same as the inputs in the tbody
    • the "send to editor" button, for vertical centering
    • remove floats on the select in the table nav, because why would you want that anyways?
  • inline docs

Why

To hopefully make this more maintainable.

For #63.

@benlk benlk added this to the 0.3.2 milestone Aug 13, 2015
@benlk
Copy link
Collaborator Author

benlk commented Aug 13, 2015

Do we want tests for this?

@rnagle
Copy link
Contributor

rnagle commented Aug 14, 2015

@benlk Yes, please, to tests.

Also, in testing this, I discovered a bug when trying to publish a link roundup. After clicking the "publish" button, the page redirects to a "Are you sure you want to do this?" message. The link roundup post remains a draft when I navigate back to the post list page in the dashboard.

@benlk
Copy link
Collaborator Author

benlk commented Aug 18, 2015

I'm going to close this PR until this is mergeable, because the WIP tests are breaking actual functionality. The file isn't testable unless the WordPress admin isn't loaded, because the WordPress admin when loaded defines ABSPATH and the testing suite has already defined that. :/

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

Successfully merging this pull request may close these issues.

2 participants