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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

Add title, classes, and icon for action in saved version #7207

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/Forms/FormAction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ class FormAction extends FormField
*/
protected $validationExempt = false;

protected $savedTitle = null;
protected $savedClasses = null;
protected $savedIcon = null;

/**
* Create a new action button.
*
Expand All @@ -93,9 +97,45 @@ public function getSchemaDataDefaults()
$defaults = parent::getSchemaDataDefaults();
$defaults['attributes']['type'] = $this->getUseButtonTag() ? 'button' : 'submit';
$defaults['data']['icon'] = $this->getIcon();
$defaults['data']['savedTitle'] = $this->getSavedTitle();
$defaults['data']['savedClasses'] = $this->getSavedClasses();
$defaults['data']['savedIcon'] = $this->getSavedIcon();
return $defaults;
}

public function setSavedTitle($title)
Copy link
Contributor

@tractorcow tractorcow Jul 26, 2017 •
edited
Loading

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 •
edited
Loading

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.

{
$this->savedTitle = $title;
return $this;
}

public function getSavedTitle()
{
return $this->savedTitle;
}

public function setSavedClasses($classes)
{
$this->savedClasses = $classes;
return $this;
}

public function getSavedClasses()
{
return $this->savedClasses;
}

public function setSavedIcon($icon)
{
$this->savedIcon = $icon;
return $this;
}

public function getSavedIcon()
{
return $this->savedIcon;
}

/**
* Get button icon, if supported
*
Expand Down