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

feature_PRESIDECMS-1047 #438

Merged
merged 3 commits into from Jun 15, 2018

Conversation

@cfmitrah
Copy link
Contributor

commented Mar 23, 2018

Added the yesNoSwitch for staying on the edit page, while editing the page from administrator.

@DominicWatson
Copy link
Contributor

left a comment

Thanks for this @cfmitrah! Logic looking great, have added some review comments for tidying some things up.

@@ -264,6 +264,7 @@ component extends="preside.system.base.AdminHandler" {
prc.page = _getPageAndThrowOnMissing( argumentCollection=arguments, allowVersions=true );
prc.canPublish = _checkPermissions( argumentCollection=arguments, key="publish", pageId=pageId, throwOnError=false );
prc.canSaveDraft = _checkPermissions( argumentCollection=arguments, key="saveDraft", pageId=pageId, throwOnError=false );
rc._backToEdit = cookie.sitetree_editPage_backToEdit ?: false;

This comment has been minimized.

Copy link
@DominicWatson

DominicWatson Mar 23, 2018

Contributor

I'd suggest the following here is more robust:

rc._backToEdit = IsTrue( cookie.sitetree_editPage_backToEdit ?: "" );

IsTrue() is a preside helper function for evaluating truthy values.

This comment has been minimized.

Copy link
@DominicWatson

DominicWatson Mar 23, 2018

Contributor

Also, please use cookieService to get / set cookie variables:

rc._backToEdit = IsTrue( cookieService.getVar( "sitetree_editPage_backToEdit", "" ) );

// ...

cookieService.setVar( name="sitetree_editPage_backToEdit", value=true );
if ( _isManagedPage( page.parent_page, page.page_type ) ) {
setNextEvent( url=event.buildAdminLink( linkto="sitetree.managedChildren", querystring="parent=#page.parent_page#&pageType=#page.page_type#" ) );
cookie.sitetree_editPage_backToEdit = false;
if ( Val( event.getValue( name="_backToEdit", defaultValue=0 ) ) ) {

This comment has been minimized.

Copy link
@DominicWatson

DominicWatson Mar 23, 2018

Contributor

We now prefer just using rc scope rather than event.getValue(). So:

if ( IsTrue( rc._backToEdit ?: "" ) ) {
// ...
}

(There is some older code that uses event.getValue(), but the elvis operator makes it cleaner to just use rc).

@@ -302,6 +302,7 @@ sitetree.add.another=Add another
sitetree.pageAdded.confirmation=Page added successfully
sitetree.editPage.title=Edit page, '{1}'
sitetree.editPage.crumb=Edit page
sitetree.editPage.backToEdit=I want to edit the page, after saving.

This comment has been minimized.

Copy link
@DominicWatson

DominicWatson Mar 23, 2018

Contributor

Perhaps: "Keep me on this page" is cleaner?

@cfmitrah

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2018

@DominicWatson Made the changes as per you comments above :)

@DominicWatson DominicWatson merged commit b938f66 into pixl8:stable Jun 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.