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 title, classes, and icon for action in saved version #7207

Closed

Conversation

phalkunz
Copy link
Contributor

return $defaults;
}

public function setSavedTitle($title)
Copy link
Contributor

@tractorcow tractorcow Jul 26, 2017

Choose a reason for hiding this comment

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

None of these methods describe what each of these are used for; They also seem to be very specific use cases baked into the generic form action class, which feels a little over-zealous.

You could generalise it, so you don't need so many extra methods into a saved state?

FormAction::create('save', _t(__CLASS__.'.SAVE', 'Save'))
    ->setIcon('save')
    ->setSavedState([
    	'title' => _t(__CLASS__.'.SAVED', 'Saved'),
    	'icon' => 'tick',
    	'extraClass' => 'primary-outline',
   	]);

and in js

getProps() {
	let props = Object.assign({}, this.props, this.props.data);
	if (!this.changed) {
		props = Object.assign({}, this.props.data.savedState);
	}
	return props;
}

Or something :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tractorcow Thanks :) Will fix it now

Copy link
Contributor

@flamerohr flamerohr Jul 27, 2017

Choose a reason for hiding this comment

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

I don't recommend mixing this.props and this.props.data like that getProps() suggestion... then the this.props.data.savedState for the changed condition makes it even worse to debug with.
if the data cannot be clearly discovered in a given path and needs to be explored in multiple areas, maybe the API needs to be re-assessed.

@tractorcow
Copy link
Contributor

For context, this is what we do in SiteTree at the moment (entwine styles)

FormAction::create('save', _t(__CLASS__.'.BUTTONSAVED', 'Saved'))
                    ->addExtraClass('btn-secondary-outline font-icon-check-mark')
                    ->setAttribute('data-btn-alternate', 'btn action btn-primary font-icon-save')
                    ->setUseButtonTag(true)
                    ->setAttribute('data-text-alternate', _t('SilverStripe\\CMS\\Controllers\\CMSMain.SAVEDRAFT', 'Save draft'))

@flamerohr
Copy link
Contributor

Closing, as the issue has been solved

@flamerohr flamerohr closed this Aug 7, 2017
@flamerohr flamerohr deleted the pulls/4.0/create-success-msg branch August 7, 2017 03:32
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.

3 participants