joomla-projects / gsoc21_frontend-inline-editing

GNU General Public License v2.0
7 stars 2 forks source link

Refactoring of the FE Inline Editing - One Case for All Fields #8

Open anibalsanchez opened 3 years ago

anibalsanchez commented 3 years ago

Problem identified

At this point, we have the sample code to do the FE Inline Editing of the article title, article text area and custom fields. We have to refactor to move the code into traits and interfaces to make it re-usable and extensible at the CMS Level.

The following cases describe the situations that a developer could found using the FE Inline Editing. These would be the ideal cases for an extension developer:

Case 1. The Simplest FE Inline Editing

I'm an extension developer of an extension. The extension has a form with several fields. One of the fields is a textarea. The extension has a form view, a controller and a model that saves the form.

I want to make the textarea editable.

I can add declarations to the controller. The model already knows how to save the data, so nothing has to be added here. The view is already rendering a form with fields, so nothing has to be added either. I don't want to manually add methods in the controller or the model or the view for the text area inline editing.

Class MyController implements InlineEditingControllerInterface {
    // The trait adds the `saveInline` method
    use InlineEditingSaveTrait;
}

saveInline does the following steps:

The field is declared in this way in the configuration form XML:

<field
    name="mycustomtextarea"
    type="textarea"
    label="JFIELD_MY_CUSTOM_TEXTAREA_LABEL"
    rows="3"
    cols="30"
    inline-editing="true"
/>

In the front-end, the field is generated in the view tmpl in this way:

<?php

// If the user can edit, it shows the value and the form.
echo InlineEditingHtmlView::render($this->item, 'mycustomtextarea', $this->item->mycustomtextarea);

?>

Case 2. FE Inline Editing and Custom Validation

Removed, check the comment for Case 2.

In addition to the previous case, the textarea has a custom validation. It has to start with a number.

The development wants to re-use the previous definitions and only add the custom validation. This would be the validation of the FE field editing:

Class MyController implements InlineEditingControllerInterface, InlineEditingValidationInterface {
    use InlineEditingSaveTrait;

    protected function validateInlineEditing(Request $request) {
        ...

        if ($field->startsWithNumber()) {
            return $this->saveInline($request);
        }

        throw new Exception("It doesn't start with a number");
        ...
    }
}

Case 3. FE Inline Editing and a Custom Field

The custom field is one more field of the form. Since the custom fields are also saved when the controller and model save the record (it triggers the event that reaches the associated field plugin), the case is supported with the same solution of Case 1 InlineEditingSaveTrait / saveInline.

Case 4. FE Inline Editing, a Custom Field with a Custom Rendering for Inline Editing

On top of the previous case, the textarea is now a custom field mycustomtextarea with a JavaScript validation to check if the field starts with a number.

The MyCustomTextarea field plugin re-uses the class Joomla\CMS\Form\Field\TextareaField to generate the field.

<field
    name="mynumberedtext"
    type="mycustomtextarea"
    label="JFIELD_MY_NUMBERED_TEXT_LABEL"
    rows="3"
    cols="30"
    inline-editing="true"
/>
class MyCustomTextareaField implements InlineEditingFormFieldInterface extends FormField
{
    ...
    /**
     * Method to get the data to be passed to the layout for the rendering of FE Inline Editing.
     *
     * @return  array
     *
     * @since 4.0
     */
    protected function getInlineEditingData()
    {
        return ['data-param1' => '....', 'data-param2' => '....'];
        }
    ...
}

Proposed solution

The traits, interfaces and function names are only examples for educational purposes. Feel free to change them to follow a different convention.

The field rendering is based on Joomla\CMS\Form\Field\TextareaField, getInlineEditingData method.

class TextareaField extends FormField implements InlineEditingFormFieldInterface
{
    protected function getInlineEditingData()
    {
        return ['data-param1' => '....'];
    }
}

FormField manages the field rendering in Joomla\CMS\Form\FormField, getLayoutData methods. The getInput and renderField methods do the following steps to generate the output:

abstract class FormField
{
    ...
        protected function getInput()
        {
            if (empty($this->layout))
            {
                throw new \UnexpectedValueException(sprintf('%s has no layout assigned.', $this->name));
            }

            $data = $this->getLayoutData();

            if ($this instanceof InlineEditingFormFieldInterface && $this->enabledInlineEditing)
            {
                $data = array_merge($this->getInlineEditingData(), $data);
            }

            return $this->getRenderer($this->layout)->render($data);
        }
    ...

    public function renderField($options = array())
    {
        ...
            $data = array_merge($this->getLayoutData(), $data);

            if ($this instanceof InlineEditingFormFieldInterface && $this->enabledInlineEditing))
            {
                        $data = array_merge($this->getInlineEditingData(), $data);
            }
        ...
    }

    ...
}

The field rendering is finally executed in layouts/joomla/form/field/textarea.php. The field already supports data-* attributes in this way:

/**
 * Layout variables
 * -----------------
 ...
 * @var   string   $dataAttribute   Miscellaneous data attributes preprocessed for HTML output
 * @var   array    $dataAttributes  Miscellaneous data attribute for eg, data-*.
 */
...

$attributes = array(
    $columns ?: '',
    $rows ?: '',
    !empty($class) ? 'class="form-control ' . $class . $charcounter . '"' : 'class="form-control' . $charcounter . '"',
    !empty($description) ? 'aria-describedby="' . $name . '-desc"' : '',
    strlen($hint) ? 'placeholder="' . htmlspecialchars($hint, ENT_COMPAT, 'UTF-8') . '"' : '',
    $disabled ? 'disabled' : '',
    $readonly ? 'readonly' : '',
    $onchange ? 'onchange="' . $onchange . '"' : '',
    $onclick ? 'onclick="' . $onclick . '"' : '',
    $required ? 'required' : '',
    !empty($autocomplete) ? 'autocomplete="' . $autocomplete . '"' : '',
    $autofocus ? 'autofocus' : '',
    $spellcheck ? '' : 'spellcheck="false"',
    $maxlength ?: '',
    !empty($counterlabel) ? $counterlabel : '',
    $dataAttribute,
);
?>
<textarea name="<?php
echo $name; ?>" id="<?php
echo $id; ?>" <?php
echo implode(' ', $attributes); ?> ><?php echo htmlspecialchars($value, ENT_COMPAT, 'UTF-8'); ?></textarea>

Finally, the textarea field plugin is configured to support the "FE Inline Editing", so the mycustomtextarea could also be implemented based on the core textarea field plugin:

File: plugins/fields/textarea/params/textarea.xml

<?xml version="1.0" encoding="utf-8"?>
<form>
    <fields name="fieldparams">
        <fieldset name="fieldparams">
        ...
            <field
                name="inline_editing"
                type="radio"
                label="PLG_FIELDS_TEXTAREA_PARAMS_INLINE_EDITING_LABEL"
                default="1"
                filter="integer"
                >
                <option value="0">JNO</option>
                <option value="1">JYES</option>
            </field>
        ...
        </fieldset>
    </fields>
</form>

Open questions

Can the current code be refactored into this organization?

CC @bembelimen @roland-d

rs4231199 commented 3 years ago

@anibalsanchez

Case 1:

Case 2:

It is the same field in the front-end too. Why would the users want to have extra validation for the inline editing? I think the validation should not be inline editing specific. We are using the default save() and that should be enough.

rs4231199 commented 3 years ago
<div class="page-header">
    <h1 itemprop="headline">
    Australian Parks
    </h1>
</div>

This is the current HTML code for the Article title. I mean this is what we get on the client-side. Could you explain the browser HTML output if we follow the above-mentioned approach? The JavaScript code will use this code to make the right request.

@anibalsanchez

anibalsanchez commented 3 years ago

@rs4231199 Let's say by some means the data stored in database doesn't pass the validation

No. Our systems always assume that the information stored in the database is valid. So, you don't have to consider the case of an inconsistent database. If you load a model from the database, then it is validated..

If the information stored in the database is not consistent, then you are developing a system to support a database that isn't SQL or doesn't have the ACID properties of transactions (atomicity, consistency, isolation, and durability).

If the developer changed the rules, then it is the developer responsibility to migrate the database to a new consistent state.

anibalsanchez commented 3 years ago

@rs4231199 Are we putting inline-editing=true in the xml for $this->enabledInlineEditing?

Yes and No. enabledInlineEditing() takes into account the inline-editing attribute but there are also other conditions that can disable the FE editing routines. For instance, the form is loaded on the backend (so the FE inline editings doesn't have to step in), FE inline editing could be globally disabled or the user doesn't have permission to the FE inline editing.

anibalsanchez commented 3 years ago

@rs4231199 echo InlineEditingHtmlView::render($this->item, 'mycustomtextarea', $this->item->mycustomtextarea);

Yes.

If enabledInlineEditing() is false, then the InlineEditingHtmlView::render outputs $this->item->mycustomtextarea.

If enabledInlineEditing() is true, then the InlineEditingHtmlView::render outputs all the necessary code to generate the required form to do the inline editing. For instance:

    <h1 itemprop="headline">
       <div class="inline-editing" >
        <div class="inline-editing-actual_value" >Australian Parks</div>
        <input data-inline-editing-action="..." data-inline-editing-method="POST" data-inline-editing-id="..." data-inline-editing-name="..." style="display:none">
        </div>
    </h1>

Please, take note that the InlineEditingHtmlView::render could execute the inline editing form generation for each field and generate the forms automatically to create something like:

    <h1 itemprop="headline">
       <form class="inline-editing" action="...." method="POST" id="..." name="...">
        <div class="inline-editing-actual_value" >Australian Parks</div>
            <input ... style="display:none">
        </form>
    </h1>

You can create a helper InlineEditingFormFieldGenerator and call it from InlineEditingHtmlView::render. Look into the following code that generates the form for the front-end article creation form: components/com_content/src/View/Form/HtmlView.php. You could load the form, get the inline-editing enabled field mycustomtextarea and generate the form for one field only, re-using the current form generation logic.

anibalsanchez commented 3 years ago

Case 2

@rs4231199 Yes, you are right. I've just added too many traits and interfaces and overengineered the idea of custom validation. I'm going to remove it.

anibalsanchez commented 3 years ago

@rs4231199 One more idea about the inline-enabled field layout.

The current layout of a field, let's say textarea is layouts/joomla/form/field/textarea.php.

....
<textarea name="<?php
echo $name; ?>" id="<?php
echo $id; ?>" <?php
echo implode(' ', $attributes); ?> ><?php echo htmlspecialchars($value, ENT_COMPAT, 'UTF-8'); ?></textarea>
...

Since you are going to be generating a textarea with data-* attributes or a form with the actual value and a textarea, there is a chance that it is too much for the regular textarea.php or other fields could require a very different output from the standard form field, so it could be a good idea to have the option of alternative layouts for inline editing. Let's say layouts/joomla/form/field/inline-editing-textarea.php.

roland-d commented 3 years ago

Here are my thoughts in random order

  1. The word edition should be editing
  2. <input data-inline-edition-action="..." data-inline-edition-method="POST" data-inline-edition-id="..." data-inline-edition-name="..." style="display:none"> We should remove the method and always doe a POST, is there any reason not to do a POST?

Can the current code be refactored into this organization?

I believe you have a good concept here and I would just run with it as-is and see if we need to fine-tune once we have actual code. I see no obvious red flags.

rs4231199 commented 3 years ago

@anibalsanchez

Please, take note that the InlineEditingHtmlView::render could execute the inline editing form generation for each field and generate the forms automatically to create something like:

    <h1 itemprop="headline">
       <form class="inline-editing" action="...." method="POST" id="..." name="...">
      <div class="inline-editing-actual_value" >Australian Parks</div>
          <input ... style="display:none">
        </form>
    </h1>

If we follow the form approach, can we have something like this?

<div class="com-content-article item-page" itemscope="" itemtype="https://schema.org/Article">
    <form class="inline-editing" action="...." method="POST" id="..." name="...">
        ...
        <div class="page-header">
            <h1 itemprop="headline">
                <div class="inline-editing-actual_value" >Australian Parks</div>
                <input ... style="display:none">
            </h1>
        </div>
        ...
    </form>
</div>

Because form action, id, name, and method are the same for all the inline editable elements inside this div.

anibalsanchez commented 3 years ago

@roland-d 1. I've just replaced Edition => Editing

  1. Yes, the data-method is not necessary. It is always a POST.
anibalsanchez commented 3 years ago

I've replaced Edition x Editing everywhere to avoid any future confusion between terms. From now on, Inline Editing

rs4231199 commented 3 years ago

@anibalsanchez We are moving forward with the form creation instead of adding data-attributes.

For the form creation we can take help of the HTMLHelper, or we can create new helpers in com_component/src/Helper. It would return something like,

<form action="<?php echo Route::_('index.php?option=com_content&a_id=' . (int) $this->item->id); ?>" method="POST">
    <div class="inline-editing-content">
        <?php echo $this->escape($this->item->title); ?>
    </div>
    <div class="inline-editing-input-field d-none">
        <?php echo $this->form->renderField('title'); ?>
    </div>
    <input name="task" value="<?php echo $task ?>" class="d-none">
        <?php echo HTMLHelper::_('form.token'); ?>
</form>
anibalsanchez commented 3 years ago

@rs4231199 Looking good. Please, go ahead and refactor the code. After we have it working, we can continue polishing it to reach the final organization.

rs4231199 commented 3 years ago

Case 1. The Simplest FE Inline Editing

In the front-end, the field is generated in the view tmpl in this way:

<?php

// If the user can edit, it shows the value and the form.
echo InlineEditingHtmlView::render($this->item, 'mycustomtextarea', $this->item->mycustomtextarea);

?>

@anibalsanchez One more question for the case 1. We need to render baseURL(url of the article), a_id, option, and the task. How will the InlineEditingHtmlView will get these two?

roland-d commented 3 years ago

@rs4231199 The base URL is available in the Joomla JS options. You can get that via Joomla.getOptions('system.paths').baseFull

We spoke in the chat about retrieving the fields when necessary so we don´t have a page full of forms. Are you taking a different approach now?

rs4231199 commented 3 years ago

I was thinking of adding empty forms. And adding form field on onClick event via Ajax request. Forms can be replaced with data-attributes. We do need a_id, option, and the task in both approaches.

Rendering empty forms(without form fields) have some advantages.

  1. We don't need to change the rendering of any existing field. The Form will have the URL as action.
  2. To perform the client-side validation, we need to add the forms eventually.
  3. JavaScript code can simulate the submitForm event which can be intercepted to modify if needed.
  4. If not the forms then we will need to use a div to encapsulate the inline-editable content and rendered input field(from the Ajax request).