Replace remove block button with dropdown that can display multiple actions

[niklasnatter]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	yes
Related issues/PRs	#6436
License	MIT

What's in this PR?

This PR replaces the remove block button that is displayed in the upper right corner of blocks with a dropdown that allows to display multiple possible actions.

Why?

This is a preparation for implementing a copy feature for blocks like described in #6436

[niklasnatter]

this matches the implementation of the download media popover and the autocomplete suggestions popover

[alexander-schranz]

The rule sounds for me something important in case of performance: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/jsx-no-bind.md

So not sure if I would remove it. I think I would go with keep it and add ignore where really required.

[niklasnatter]

i removed the rule because it makes list rendering a pain as you need to extract a component for the list item. in this case, i think the rule makes the code even slower because we need to instantiate a whole component for each item instead of just an arrow function 🙈

but yeah, i guess there are cases where it improves performance too. i will readd it.

[alexander-schranz]

we need to instantiate a whole component for each item instead of just an arrow function

I see, but theoretically the component is initialized only once. And the function is created for every render so it also can in this case improve the performance. I'm just not sure how much mobx maybe would effect here things and maybe avoid rerender here something.
Atleast we also created in this cases also for Select Option own component to avoid this kind of problems. So I personally would go the save way of introducing also here a MenuOption or what the best name is here.

[alexander-schranz]

To be consistent I would create a MenuOption and or MenuItem component or what best matching name is. So we also get rid of the react/jsx-no-bind ignore here.

[niklasnatter]

i extracted a ActionPopoverItem. but it really feels cumbersome to do this, i am not a fan of the react/jsx-no-bind rule 🙈

[alexander-schranz]

a button should always define its type else it accidently gets a submit button when wrapped inside a form tag.

Suggested change

	}}
	>
	}}
	type='button'
	>

[alexander-schranz]

We could add https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/button-has-type.md to avoid this in future.

[niklasnatter]

added the rule 🙂 i guess i will need to update a lot of snapshots

[alexander-schranz]

Not sure about htis rename. Think hasMaximumReached present it better as maxOccursReached but was never a fan of the occurs word 🙈

[alexander-schranz]

Any easy way to make this better accessible?

E.g.:

<ul class="iconButtons"
   <li>
       <button ...>
           <span class="su-more-circle" aria-hidden="true"/>
           <span class="sr-only">More</span>
       </button>
   </li>
   <li>
       <button ...>
           <span class="su-collapse-vertical" aria-hidden="true"/>
           <span class="sr-only">Collapse Block</span>
       </button>
   </li>
</ul>

[niklasnatter]

this is rendered using the Icon component:

sulu/src/Sulu/Bundle/AdminBundle/Resources/js/components/Block/Block.js

Lines 100 to 106 in c298925

	<div className={blockStyles.iconButtons}>
	{onSettingsClick && <Icon name="su-cog" onClick={onSettingsClick} />}
	{onRemove && <Icon name="su-trash-alt" onClick={onRemove} />}
	{onCollapse && onExpand &&
	<Icon name="su-angle-up" onClick={this.handleCollapse} />
	}
	</div>

i can check if anything breaks if i use the Button component

[alexander-schranz]

I would not use the button component just us a button tag ;)