Problem/Motivation

Currently, we do not have default action in our paragraph_actions element but we generate it outside of this component. Because of this we also can not support multiple optional default actions in our paragraph_actions element and this is complicating our HTML/CSS for layout.

Here are a couple of screenshots explaining things more visually:

Proposed resolution

  • Add support for default actions in paragraph_actions and start using it in current code.
  • Default actions are optional - meaning if we do not have any default action then we do not show default action but just a 3 dots dropdown button.
  • We can have more then one default action in our actions - in this case we are only showing the first default action outside of dropdown buttons as a default action button, the rest we are rendering inside the dropdown.
  • Consider should we add default option for top root paragraph_actions element (Collapse All/Edit All) button.
  • The ParagraphsWidget.php has a next @todo's that needs to be resolved with this issue also: 
    • // @todo - with the ddo issue #2901549 'Introduce default actions for
// paragraph_actions dropdown' ddo issue' paragraph_actions element with
// only one default button should then render it self as simple button.
// Then we would not need this if else here but we could just simply
// call buildActionsElement().
    • // @todo - change this with #2901549 - rename hook and to
// paragraph_widget_actions and pass subarray which holds both
// default buttons and dropdown buttons.

Remaining tasks

Do it.

CommentFileSizeAuthor
#9 interdiff-2901549-6-9.txt1 KBpivica
#9 default-actions-and-refactoring-2901549-9.patch40.75 KBpivica
#6 default-actions-and-refactoring-2901549-6.patch39.75 KBpivica
#6 2901549-default-actions-and-refactoring--collapse-all-variation.png50.72 KBpivica
#6 2901549-default-actions-and-refactoring--edit-all-variation.png48.67 KBpivica
#2 2901549-collapse-as-default-action.png30.69 KBpivica
#2 2901549-edit-as-default-action.png30.32 KBpivica

Comments

pivica created an issue. See original summary.

pivica’s picture

pivica commented
Issue summary: View changes
StatusFileSize
new 2901549-edit-as-default-action.png30.32 KB
new 2901549-collapse-as-default-action.png30.69 KB
pivica’s picture

pivica commented
Issue summary: View changes
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Priority: Normal » Major

So yeah, here some facts from my side:

The fundamental wrong design of the core drop button is that it has always the first action as the default action. Sometimes this simply makes no sense.

I agree, for maximum flexibility we can merge all the action buttons into one. Actions alter code can then easily alter all buttons.

I remember, Bootstrap has button groups that contains multiple actions. We could simply list all default actions in their sequence outside the "..." and then the rest after clicking on "...".

Making collapse a default action after edit seems a good idea. I even think that button could be primary color. But i also think that some related logic could vary based on e.g. if autocollapse mode is on and default mode is closed (or edit)...

Putting to major as expand / collapse is our primary editor interaction workflow. We should create something smooth and awesome straight forward.

pivica’s picture

pivica commented
Issue summary: View changes
pivica’s picture

pivica commented
Status: Active » Needs review
StatusFileSize
new 2901549-default-actions-and-refactoring--edit-all-variation.png48.67 KB
new 2901549-default-actions-and-refactoring--collapse-all-variation.png50.72 KB
new default-actions-and-refactoring-2901549-6.patch39.75 KB

Here is a first patch, lots of things done but they are all kinda related and also connected with CSS changes that I am doing for #2868225: UI for mobile devices is not looking nice.

Changelog:

  • SASS variables are now shared among SCSS files and are in _variables.scss.
  • Single to plural fixes for paragraphs_actions element - from 'paragraph' to 'paragraphs'. Also change class name from ParagraphActions to ParagraphsActions.
  • paragraphs_actions element now host both actions (default buttons) and dropdown actions. This also allow us some code simplification.
  • paragraphs_actions element is not using attributes now for storing actions and dropdown actions but regular child arrays. This allow us to drop fully buildActionsElement() method in ParagraphsWidget.
  • Added paragraphs-actions.html.twig template.
  • Cleaned a bit HTML/CSS naming for refactored paragraphs_actions component.
  • Dropdown toggle is a bit bigger now for small screens - UX improvement.
  • 'Edit All', 'Collapse All', 'Collapse' and 'Edit' are now always actions and not dropdown actions. Header actions are converted to new paragraphs_actions element fully.
  • We are showing lock icon now for both edit and collapse widget state.
  • Alter hook 'paragraph_widget_dropbutton' is renamed to 'paragraphs_widget_actions' and we are passing array with both actions and dropdown actions there.
  • Improved moving table header actions to table header code. Related to recent change in #2903243: Fix display of header actions.

Here are a couple of screenshots that show refactored paragraphs_actions element and support for actions and dropdown_actions:

Collapse all example:

Edit all example:

Status: Needs review » Needs work

The last submitted patch, 6: default-actions-and-refactoring-2901549-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

pivica’s picture

pivica commented

We need a change in paragraphs_collection/paragraphs_library because of hook name and structure change. Here is a patch #2903793-2: Paragraphs hook_paragraph_widget_dropbutton_alter() name change..

pivica’s picture

pivica commented
Status: Needs work » Needs review
StatusFileSize
new default-actions-and-refactoring-2901549-9.patch40.75 KB
new interdiff-2901549-6-9.txt1 KB

Fixed test.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

Hm we have many implementations of that hook... would it be compatible with old data if we keep its name unchanged?

If we need to add aome new structure, we could be nice and still trigger the old and care about its return data... declare it deprecated and then continue with the new one. Then we delete it when the time is right. Is this acceptable from temporary code complexity?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

The structure is different, so not sure if that would make sense.

This is part of the experimental widget, and we've already made a bunch of changes like changing the nesting level that broke all those alter hooks anyway, one more isn't going to make a big difference now.

  • miro_dietiker committed c3d33b7 on 8.x-1.x authored by pivica 
    Issue #2901549 by pivica, miro_dietiker: Introduce default actions for...
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Fixed

OK committed. Great progress.

I would prefer to name $widget_actions in the hook just $actions.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.