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

ENH Render some buttons for blocks that can not be edited inline #963

Conversation

maxime-rainville
Copy link
Contributor

Some block types can not be edited in an Elemental Area (e.g.: UserForm blocks). Current behaviour is to not render any actions for those blocks in the Elemental Area.

However some action still make sense in that context (e.g.: Duplicate, Archive, Unpublish).

This PR allows those extra actions to be displayed.

Parent issue

Copy link
Contributor Author

@maxime-rainville maxime-rainville left a comment

Choose a reason for hiding this comment

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

Turns out we didn't the Context provider non-sense at all. We just needed to pass the expandable prop all the way down.

Copy link
Member

@emteknetnz emteknetnz left a comment

Choose a reason for hiding this comment

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

Tested locally, does what it should do. Unpublish button remains on elemental-userforms which makes sense IMO. Some minor changes requested

@@ -91,8 +92,8 @@ class ElementActions extends Component {
dropdownMenuProps={{ right: true }}
dropdownToggleClassNames={dropdownToggleClassNames}
>
{ this.renderEditTabs() }
{ this.renderDivider() }
{ expandable && this.renderEditTabs() }
Copy link
Member

Choose a reason for hiding this comment

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

should so the conditional check on this.props.expandable within renderEditTabs() and renderDivider() rather then here. Both those methods do conditional early returns on props

Copy link
Member

Choose a reason for hiding this comment

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

@maxime-rainville remember to do this one

client/src/components/ElementEditor/ElementActions.js Outdated Show resolved Hide resolved
@maxime-rainville maxime-rainville force-pushed the pulls/4/show-some-action-on-none-inline-blocks branch from 9594659 to 92affe8 Compare March 23, 2022 04:52

if (editTabs && editTabs.length && React.Children.count(children)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I swap the return null and return DropdownItem around just so the approach between renderDivider and renderTabs are consistien

Copy link
Contributor Author

@maxime-rainville maxime-rainville left a comment

Choose a reason for hiding this comment

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

I was thinking it might be nice to cover this behavior with some behat test, but we can't really do it without requiring userform and the userfrom block because that's the only block that can't be rendered inline.

@GuySartorelli
Copy link
Member

GuySartorelli commented Mar 23, 2022

we can't really do it without requiring userform and the userfrom block because that's the only block that can't be rendered inline.

I haven't done anything with behat tests yet so excuse my ignorance but I take it then that behat tests can't take advantage of classes that implement TestOnly?
Or temporary config changes for that matter? Since whether a block renders inline or not is controlled by config, if you could change that config for the duration of the test on say the content block, that would allow such a test to work.

@emteknetnz emteknetnz merged commit f8103d1 into silverstripe:4 Mar 23, 2022
@emteknetnz emteknetnz deleted the pulls/4/show-some-action-on-none-inline-blocks branch March 23, 2022 21:46
@maxime-rainville
Copy link
Contributor Author

I haven't done anything with behat tests yet so excuse my ignorance but I take it then that behat tests can't take advantage of classes that implement TestOnly?

@GuySartorelli Exactly ... your behat test need to be run against an actual functional website. Occasionally, we'll install frameworktest so we can test specific features that are not directly used in our core modules.

@emteknetnz
Copy link
Member

emteknetnz commented Mar 29, 2022

@GuySartorelli example behat equivalent of a TestOnly page - https://github.com/silverstripe/silverstripe-frameworktest/blob/4/code/multitab-validation/MultiTabPage.php

It's a kind of janky solution having to include frameworktest given that moudles intended use is different (generating lots of dumy pages and dataobjects), though it practice it's good enough for our needs

menno-rdmkr pushed a commit to busting-bytes/silverstripe-elemental that referenced this pull request Dec 15, 2022
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.

Some of the "More Actions" should still show on non in-line editable blocks
3 participants