Port betterbuttons to framework #8569

bergice · 2018-11-05T09:25:40Z

This adds Previous, Next and Add buttons to the edit form. It should also retain the search filters from the grid field where it was opened from.

Fixes #436

_config/buttons.yml

unclecheese

Nice work. Biggest thing is I see a lot of artefacts from BetterButtons that don't apply here, such as owner and Controller::curr(), in addition to a number of other syntactical issues.

Second major thing is these will need unit tests.

Otherwise, I think you've done a good job of containing the story and not blowing up the API too much in the interest of parity with BetterButtons. We can improve upon this in a future story.

src/Forms/GridField/GridFieldDetailForm_ItemRequest.php

src/Forms/GridField/GridFieldEditButton.php

_config/buttons.yml

src/Forms/GridField/GridFieldDetailForm_ItemRequest.php

Rectified all the requested changes.

unclecheese

Getting there. High level points:

Change showNext and showPrev to a monolithic showPagination
Refactor getPreviousID() and getNextID() to be more scalable. Current implementation is inherently flawed.

docs/en/02_Developer_Guides/15_Customising_the_Admin_Interface/How_Tos/Customise_CMS_Menu.md

src/Forms/GridField/GridFieldDetailForm.php

src/Forms/GridField/GridFieldDetailForm_ItemRequest.php

unclecheese · 2018-11-12T23:49:53Z

src/Forms/GridField/GridFieldDetailForm_ItemRequest.php

+    {
+        Controller::curr()->getResponse()->addHeader("X-Pjax", "Content");
+        $link = $this->getEditLink($this->getNextRecordID());
+        return Controller::curr()->redirect($link);


getResponse() isn't available on RequestHandler. Let's use $this->getTopLevelController()->getResponse() and $this->redirect().

src/Forms/GridField/GridFieldDetailForm_ItemRequest.php

unclecheese · 2018-11-14T20:58:45Z

The changes to GridFieldPaginator are likely incompatible with the #1 installed module in the SS ecosystem, gridfieldextensions, which offers a GridFieldConfigurablePaginator component that uses its own itemsPerPage state.

We haven't broken any APIs, so this is safe to merge into a minor release, but I think we should at least check in with the maintainers to let them know about this change so they can issue a 4.4.x compatible release. CC @robbieaverill @NightJar

robbieaverill · 2018-11-14T21:25:30Z

Thank for checking @unclecheese, we'll test it out. Hopefully nothing breaks, it might get a little hairy to adjust the code to support both versions if it does - we'll see =)

If the form is opened via a grid field, the filters will be retained so the previous/next record opened will be correct

…xt` and `Add` buttons at a `GridField` level

…ess `betterbuttons` module was installed

…elds - Update documentation - Improve performance for next/previous buttons by not fetching all list records - Refactoring

unclecheese · 2018-11-18T21:52:43Z

Awesome. Tests were failing because $state->getData('GridFieldPaginator') wasn't always defined. Have put a guard in against that.

Other minor points:

Consistency with single/double quotes. Prefer single, unless doing variable interpolation
Private members should be used sparingly, as they're antithetical to extensibility.
Don't assume config vars are set ($formConfig['showAdd']), as it tightly couples your code to the config.

Merge on green.

robbieaverill · 2018-11-19T11:17:11Z

Consistency with single/double quotes. Prefer single, unless doing variable interpolation

We should add this to phpcs rules. Related: #8343

bergice requested a review from unclecheese November 5, 2018 09:26

chillu mentioned this pull request Nov 5, 2018

Prev, Next and Create buttons on detail GridField forms silverstripe/silverstripe-admin#436

Closed

16 tasks

sminnee reviewed Nov 5, 2018

View reviewed changes

_config/buttons.yml Outdated Show resolved Hide resolved

bergice force-pushed the pulls/4/436-port-betterbuttons branch from 8a8a0dc to a9f3c77 Compare November 5, 2018 22:41

unclecheese previously requested changes Nov 5, 2018

View reviewed changes

unclecheese force-pushed the pulls/4/436-port-betterbuttons branch from a9f3c77 to 0c08821 Compare November 5, 2018 23:34

bergice force-pushed the pulls/4/436-port-betterbuttons branch 4 times, most recently from c68d5a7 to 1759224 Compare November 7, 2018 05:17

bergice force-pushed the pulls/4/436-port-betterbuttons branch from 6e20bda to 2ae244b Compare November 12, 2018 05:14

unclecheese requested changes Nov 13, 2018

View reviewed changes

robbieaverill mentioned this pull request Nov 14, 2018

Check for compatibility with new better buttons changes in SS 4.4.x symbiote/silverstripe-gridfieldextensions#277

Open

bergice and others added 5 commits November 19, 2018 10:49

MINOR: Add Previous, Next and Create New actions in edit form

1a9b893

If the form is opened via a grid field, the filters will be retained so the previous/next record opened will be correct

MINOR: Add ability to customise the visibility of the Previous, `Ne…

62aab62

…xt` and `Add` buttons at a `GridField` level

Fix invalid action when pressing the New button in an edit form unl…

a187553

…ess `betterbuttons` module was installed

- Merge showPrevious and showNext to showPagination for grid fi…

e8dc2f1

…elds - Update documentation - Improve performance for next/previous buttons by not fetching all list records - Refactoring

Refactor to fail gracefully on GridFieldPaginator

d990ecc

unclecheese force-pushed the pulls/4/436-port-betterbuttons branch from 77fba55 to d990ecc Compare November 18, 2018 21:49

unclecheese approved these changes Nov 18, 2018

View reviewed changes

unclecheese merged commit cc71289 into silverstripe:4 Nov 18, 2018

unclecheese deleted the pulls/4/436-port-betterbuttons branch November 18, 2018 22:06

robbieaverill mentioned this pull request Nov 19, 2018

Tool to maintain repo and github metadata across all supported modules #8343

Closed

bergice restored the pulls/4/436-port-betterbuttons branch November 19, 2018 22:29

bergice mentioned this pull request Nov 22, 2018

NEW Allow state to be passed back on grid field actions silverstripe/silverstripe-admin#757

Merged

robbieaverill added change/minor affects/v4 labels Nov 26, 2018

robbieaverill mentioned this pull request May 1, 2019

API Replace FormActions with anchors to enable panel-based loading in GridField navigation buttons #8953

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port betterbuttons to framework #8569

Port betterbuttons to framework #8569

bergice commented Nov 5, 2018

unclecheese left a comment

unclecheese left a comment

unclecheese Nov 12, 2018

unclecheese commented Nov 14, 2018 •

edited

Loading

robbieaverill commented Nov 14, 2018

unclecheese commented Nov 18, 2018

robbieaverill commented Nov 19, 2018

Port betterbuttons to framework #8569

Port betterbuttons to framework #8569

Conversation

bergice commented Nov 5, 2018

unclecheese left a comment

Choose a reason for hiding this comment

unclecheese left a comment

Choose a reason for hiding this comment

unclecheese Nov 12, 2018

Choose a reason for hiding this comment

unclecheese commented Nov 14, 2018 • edited Loading

robbieaverill commented Nov 14, 2018

unclecheese commented Nov 18, 2018

robbieaverill commented Nov 19, 2018

unclecheese commented Nov 14, 2018 •

edited

Loading