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

Add hook to allow adding buttons to page listing #1940

Closed

Conversation

mx-moth
Copy link
Contributor

@mx-moth mx-moth commented Nov 19, 2015

The 'register_page_listing_buttons' hook allows plugin developers to add buttons to the actions list on the page listing in the admin.

This will come in useful when adding the revision rollback feature in the future, as the rollback plugin can add a button to the page listing.

Does this concept sound like a good plan to you? Documentation and tests will be coming shortly.

@davecranwell
Copy link
Contributor

I like the principal of users being able to add their own buttons in situations like this, but fwiw I'm not a fan of adding more buttons to the explorer specifically.

At present there are already buttons in the explorer allowing you to do things to a page which aren't available within the page editor itself, which is a mistake in my opinion.

If you edit an image, you expect to be able to control the image entirely through the image editor interface. If you edit a document, you expect to be able to do everyhting you need through the document editor interface. If you edit a page however, you can't copy it, move it, add a child, or view the draft without using the awkward preview system, from within the page itself.

While we may need to add a revisions button in the explorer temporarily I'm wary of giving users the ability to scatter about action buttons who's locations ought to be more predictable.

@mx-moth
Copy link
Contributor Author

mx-moth commented Nov 19, 2015

I hear what your saying, where buttons appear is a little confusing and inconsistent currently. One (possibly silly) solution: just add this button row to the edit screen as well. Then which buttons appear are consistent across all instances of the button row, everywhere.

@davecranwell
Copy link
Contributor

That's certainly an option yes, although that would be inconsistent with the header areas of other entities in Wagtail and the page editor header is already creaking and poorly aligned.

What i'm really after is a common UI paradigm on all entities, something more like the right-hand column on the image and document editing pages, in which users can reliably expect to find the Buttons Wot Do Stuff.

@gasman gasman added this to the 1.3 milestone Nov 20, 2015
@mx-moth
Copy link
Contributor Author

mx-moth commented Nov 22, 2015

A right-hand column of Buttons Wo Do Stuff would not play nice with the current full-bleed interface for some fields. I know there are plans to redo this part of the UI, and I am not much of a UI designer myself, so I dont want to step on any toes here. If I make a 'register_button_wot_does_stuff ' hook, could we then discuss how to move forward implementing the UI aspect of this?

@davecranwell
Copy link
Contributor

Yeah definitely go ahead with the hook. We can figure the rest out later.

Also on the topic of stepping on toes, we don't have anyone assigned to this task yet, so any suggestions or wireframes you feel up to creating would be welcomed :)

@gasman
Copy link
Collaborator

gasman commented Nov 23, 2015

For the revisions feature, I think the plan was to link it from the "last edited by..." text in the footer of the page editor, rather than adding any extra buttons. (I think we really need a proper written spec for revisions at this point, to stop people going up the garden path...)

@mx-moth mx-moth force-pushed the feature/page-listing-buttons branch 3 times, most recently from ec7e40b to 76a2ebe Compare December 9, 2015 01:39
@mx-moth mx-moth force-pushed the feature/page-listing-buttons branch 2 times, most recently from eeb176e to 5ef26d4 Compare December 13, 2015 22:06
buttons = sorted(itertools.chain.from_iterable(
hook(page, page_perms, is_parent)
for hook in button_hooks))
print(buttons)
Copy link
Contributor

Choose a reason for hiding this comment

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

Stray print()!

@gasman gasman modified the milestones: 1.4, 1.3 Dec 17, 2015


@hooks.register('register_page_listing_buttons')
def page_listing_buttons(page, page_perms, is_parent=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice if the hook didn't have to care about where/how the buttons are displayed so third party apps wouldn't have to be updated if we ever decide to change where to display the buttons.

Probably not high priority for now, but maybe a more abstract concept (like django's admin actions) might be worth looking into if that become a problem.

@mx-moth mx-moth force-pushed the feature/page-listing-buttons branch from 5ef26d4 to 2d8d991 Compare January 25, 2016 05:18
@nkuttler
Copy link
Contributor

nkuttler commented Feb 3, 2016

I just found this pr because I have exactly the same problem. In my case I just want an additional button for my private website, and there's no way to add it without overriding almost the entire admin template. I think the edit.html with the page editor should have a similar mechanism for adding actions. I'll keep an eye on this pull request and override entire admin templates for now.

@mx-moth mx-moth force-pushed the feature/page-listing-buttons branch 2 times, most recently from fbe027e to 58ffb66 Compare February 4, 2016 23:48
@tomdyson
Copy link
Contributor

Here's @JoshBarr's mockup of an idea to solve the too-many-buttons problem.

josh-extended-buttons-mockup

@kaedroho
Copy link
Contributor

kaedroho commented Mar 1, 2016

This is missing the required CSS changes for the new dropdown. Could you add these?

@mx-moth
Copy link
Contributor Author

mx-moth commented Mar 2, 2016

OK, this has got the styles from @JoshBarr now. There is a small issue with button sizes in the list of buttons in Firefox, in that they are all different heights, although they only differ by ±1px. I moved the hard-coded example list of traffic light buttons to the styleguide, but the styles break a little when moved out of their current location. At least the styles/html is preserved for future reference, even if the look is not exactly correct.

screenshot-wagtail-buttons

@tomdyson
Copy link
Contributor

tomdyson commented Mar 2, 2016

Hi @m1kola could you please review this? @JoshBarr will look at the CSS issues separately.

@m1kola
Copy link
Contributor

m1kola commented Mar 2, 2016

@tomdyson I will look at this tomorrow evening. Is it ok?

@gasman
Copy link
Collaborator

gasman commented Mar 8, 2016

Note to whoever ends up resolving merge conflicts: please take care not to lose the new .warning button styles added to _forms.scss in e782de6.

@gasman
Copy link
Collaborator

gasman commented Mar 8, 2016

Rebased on master at https://github.com/gasman/wagtail/tree/takeflight-feature/page-listing-buttons (including @m1kola's tests neon-jungle#3).

I'm afraid I have severe accessibility concerns about the dropdown widget in its current form - if you click on the 'More...' button of a child page in the explorer, and then your mouse pointer strays outside the boundaries of the dropdown while attempting to click on one of the options, the dropdown disappears. I don't think my motor control is particularly bad, but I found it just fiddly enough to be annoying. I think we need some extra JS to make the menu (and the rest of the row's buttons) 'stick' until the user explicitly clicks outside of it. (As a more nitpicky change - I think the links on the dropdown items also need to be made block-level, i.e. cover the full width of the dropdown rather than just the text span.)

@gasman gasman assigned JoshBarr and unassigned m1kola Mar 8, 2016
@gasman gasman modified the milestones: 1.5, 1.4 Mar 9, 2016
The 'register_page_listing_buttons' hook allows plugin developers to add
buttons to the actions list on the page listing in the admin.
@JoshBarr JoshBarr force-pushed the feature/page-listing-buttons branch from f826c87 to 266fe1a Compare March 10, 2016 02:49
@JoshBarr JoshBarr force-pushed the feature/page-listing-buttons branch from 56176cb to 6989abf Compare March 10, 2016 03:09
@JoshBarr
Copy link
Contributor

@gasman i didn't realise you'd rebased this one elsewhere. I've updated the JS to address the usability issue. Would you like to add the tests to this PR, or move the JS to the new PR?

@gasman
Copy link
Collaborator

gasman commented Mar 10, 2016

@JoshBarr Thanks! This PR is looking a lot neater than my rebased branch now, so I'd suggest cherry-picking @m1kola's commit m1kola@4f1125e on to here.

// States
// =============================================================================

$color-state-live: #59b524;
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems likely we're going to want to use these colours to replace the main Explorer status tag soon. Could this be moved to the main variables.scss?

JoshBarr and others added 2 commits March 11, 2016 15:44
Tests folowing:
 * `register_page_listing_buttons`
 * `register_page_listing_more_buttons`
 * Custom hook produced by non-default `ButtonWithDropdownFromHook` instance

organised indicator color state variables into main variables.scss file
@JoshBarr JoshBarr force-pushed the feature/page-listing-buttons branch from bfc059f to f8a584e Compare March 11, 2016 02:45
@JoshBarr
Copy link
Contributor

I've cherry-picked the tests and added some documentation for this feature. Should be good to merge.

gasman added a commit to gasman/wagtail that referenced this pull request Mar 11, 2016
gasman added a commit to gasman/wagtail that referenced this pull request Mar 11, 2016
@gasman
Copy link
Collaborator

gasman commented Mar 11, 2016

Merged in 6803075 + parents.

@JoshBarr Thanks for getting those last fixes in so quickly! Sorry that the last-minute reviewing meant we weren't able to get this into 1.4 - it just missed the cutoff by a whisker - but we weren't able to postpone the release candidate any longer, as we're under a lot of pressure to get the likes of collections into a release version.

@zerolab zerolab closed this Mar 11, 2016
JoshBarr pushed a commit to springload/wagtail that referenced this pull request Apr 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants