joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

[5.2] [GSoC 21] Core Enhancement 2 - Incorporation of Modules in Articles-Edit-View

Open YatharthVyas opened this issue 4 years ago • 16 comments

Project Repo PR: https://github.com/joomla-projects/gsoc21_core-enhancements/pull/8 Plan Document: https://docs.google.com/document/d/1Pl8JGa2hkYkmJzQOn9_mS8a4imDmqc2a/edit#heading=h.xzcjfqetxfhr


Summary of Changes

  1. Create Module in Article Edit Screen
    Users can add Modules in Articles in Joomla. This is done with the help of the Module xtd-editor Plugin. However, this Plugin opens a Modal that only allows the user to select a pre-existing module, ie. there is no option to add a new module directly in the Article Edit View. To enhance that, this PR introduces a button to Create Modules directly in the Article Edit View:
    add module

  1. Imported Modules Tab
    Note: To test this feature, you must enable "Show Imported Modules" in the com_content Global Configuration OR in the Configure Edit Screen Tab. Also, remember that the tab is only visible when you Import a Module and Save it.
    configure

    • Users have no option to ascertain information about the Modules imported in an article (they can only see the module id)
      image

    • Additionally, these modules cannot be directly edited in the Article Edit View.
      edit

    • There is no user-friendly way to removing these modules. It requires a person to backspace the {loadmoduleid id} string from the article text which may not seem very intuitive to someone that is not tech-savvy. So to make this process more smooth, the users can now remove the tab with the click of a button in this tab
      remove

Testing Instructions

  1. Run npm ci to rebuild the assets
  2. Enable the Imported Modules Tab (Global Configuration -> Articles -> Editing Layout -> Show Imported Modules Switcher)
  3. Follow the GIFs above

Documentation Changes Required

I'm not sure

Mentors

@chmst @nibra @bembelimen (and thanks @richard67)

YatharthVyas avatar Jul 12 '21 13:07 YatharthVyas

When you use the NEW button why do you have to repeat everything in order to select and insert the module. This should be a single step.

brianteeman avatar Jul 12 '21 14:07 brianteeman

Something for you to review with your mentors.

Everything in this code is dependent on adding modules to the content using the syntax {loadmoduleid 1}

However modules can also be added to content using alternative syntax

  • {loadposition position_name}
  • {loadposition module_name}

Any modules present in the content using either of these alternatives will not be included in the tab containing "imported modules"

brianteeman avatar Jul 12 '21 15:07 brianteeman

However modules can also be added to content using alternative syntax

When modules are added, is does not matter. The id is unique, the name was a small help for users - but the name is not always unique.

chmst avatar Jul 12 '21 17:07 chmst

@chmst you miss the point.

Because a module can be added to an article with three different syntax (and if I remember correctly the id method was not the original) a user will expect all the modules in an article to be displayed in the imported modules tab. With the way the current code is here that will not be the case.

brianteeman avatar Jul 12 '21 17:07 brianteeman

The new commits address 2 of the improvements suggested by @brianteeman

1. Importing a newly created module is now a single-step process. No need to re-open the Modal.

https://user-images.githubusercontent.com/53610833/125674142-803bf23a-b3a9-47e4-b6b1-8013e40d9693.mp4

I've used a cookie here as I need to access its contents using both PHP (to not display the Save and Close button in Toolbar for unsaved modules) and JavaScript (to clear this flag when we close the modal). Hiding Save and Close button is necessary here because we need to make sure that the module is saved (in our database) before we close the modal so we can grab the module's ID.

2. Modules positions also shown in the imported modules tab (for loadposition):

image

Only the positions are shown and not the modules under these positions because that requires us to know the menu id for the article's page (can be in multiple pages) and then mapping the menu ID to position to modules which will be a costly performance impacting query.

Scope for Improvement

  • Modules imported using {loadmodule mod_type} are not displayed in the new tab. This should be implemented.

YatharthVyas avatar Jul 14 '21 18:07 YatharthVyas

All 3 Types of module import syntax are considered in the imported modules tab

image

The new addition here is the Modules Imported by Type Section. Here's how it works:

  • It is similar to the flow of the loadmodules plugin
  • We first check if the ModuleHelper:getModule returns a valid module
  • If not, we add mod_ as a prefix to the name and call the ModuleHelper:getModule function
  • When the moduleHelper function returns a value, we append it to the array (sometimes a ID of 0 is return even though the module exists which is why there are some modules having ID 0 and no edit button)

YatharthVyas avatar Jul 17 '21 10:07 YatharthVyas

This pull request has automatically rebased to 4.2-dev.

HLeithner avatar Jun 27 '22 13:06 HLeithner

This pull requests has been automatically converted to the PSR-12 coding standard.

joomla-bot avatar Jun 27 '22 21:06 joomla-bot

This pull request has been automatically rebased to 5.0-dev.

HLeithner avatar May 02 '23 16:05 HLeithner

I have tested this item :red_circle: unsuccessfully on dcad97f9986bf7f3c66938e977c844b12ff4d714

If I apply the patch and start a new article I get this above the edit form:

Deprecated: preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated in /Users/ceford/Sites/joomla-cms/administrator/components/com_content/src/Model/ArticleModel.php on line 462

Deprecated: preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated in /Users/ceford/Sites/joomla-cms/administrator/components/com_content/src/Model/ArticleModel.php on line 478

Deprecated: preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated in /Users/ceford/Sites/joomla-cms/administrator/components/com_content/src/Model/ArticleModel.php on line 506

Deprecated: preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated in /Users/ceford/Sites/joomla-cms/administrator/components/com_content/src/Model/ArticleModel.php on line 462

Deprecated: preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated in /Users/ceford/Sites/joomla-cms/administrator/components/com_content/src/Model/ArticleModel.php on line 478

Deprecated: preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated in /Users/ceford/Sites/joomla-cms/administrator/components/com_content/src/Model/ArticleModel.php on line 506

I can create a new module but it is not inserted in the edit form - so this feature seems pointless.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34764.

ceford avatar Sep 09 '23 13:09 ceford

This pull request has been automatically rebased to 5.1-dev.

HLeithner avatar Sep 30 '23 22:09 HLeithner

This pull request has been automatically rebased to 5.2-dev.

HLeithner avatar Apr 24 '24 09:04 HLeithner

I have tested this item :red_circle: unsuccessfully on dcad97f9986bf7f3c66938e977c844b12ff4d714

I do not know what "npm" is or how to execute it. So that aside, after installing the patch I get the following error when either editing an existing article or creating a new one. I have seen this during testing of other patches during PBF.

An error has occurred. 0 There is no "com_content.admin-article-edit" asset of a "script" type in the registry. Call Stack

Function Location

1 () JROOT/libraries/src/WebAsset/WebAssetRegistry.php:135 2 Joomla\CMS\WebAsset\WebAssetRegistry->get() JROOT/libraries/src/WebAsset/WebAssetManager.php:274 3 Joomla\CMS\WebAsset\WebAssetManager->useAsset() JROOT/libraries/src/WebAsset/WebAssetManager.php:208 4 Joomla\CMS\WebAsset\WebAssetManager->__call() JROOT/administrator/components/com_content/tmpl/article/edit.php:26 5 include() JROOT/libraries/src/MVC/View/HtmlView.php:416 6 Joomla\CMS\MVC\View\HtmlView->loadTemplate() JROOT/libraries/src/MVC/View/HtmlView.php:204 7 Joomla\CMS\MVC\View\HtmlView->display() JROOT/administrator/components/com_content/src/View/Article/HtmlView.php:135 8 Joomla\Component\Content\Administrator\View\Article\HtmlView->display() JROOT/libraries/src/MVC/Controller/BaseController.php:697 9 Joomla\CMS\MVC\Controller\BaseController->display() JROOT/administrator/components/com_content/src/Controller/DisplayController.php:65 10 Joomla\Component\Content\Administrator\Controller\DisplayController->display() JROOT/libraries/src/MVC/Controller/BaseController.php:730 11 Joomla\CMS\MVC\Controller\BaseController->execute() JROOT/libraries/src/Dispatcher/ComponentDispatcher.php:143 12 Joomla\CMS\Dispatcher\ComponentDispatcher->dispatch() JROOT/libraries/src/Component/ComponentHelper.php:361 13 Joomla\CMS\Component\ComponentHelper::renderComponent() JROOT/libraries/src/Application/AdministratorApplication.php:150 14 Joomla\CMS\Application\AdministratorApplication->dispatch() JROOT/libraries/src/Application/AdministratorApplication.php:195 15 Joomla\CMS\Application\AdministratorApplication->doExecute() JROOT/libraries/src/Application/CMSApplication.php:306 16 Joomla\CMS\Application\CMSApplication->execute() JROOT/administrator/includes/app.php:58 17 require_once() JROOT/administrator/index.php:32


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34764.

bascherz avatar Aug 24 '24 17:08 bascherz

I have tested this item 🔴 unsuccessfully on dcad97fI do not know what "npm" is or how to execute it. So that aside, after installing the patch I get the following error when either editing an existing article or creating a new one. I have seen this during testing of other patches during PBF.

@bascherz NPM is the Node Package Manager which we use to maintain our javascript dependencies and to generate the js and css from the sources. If you haven't used it and haven't used a prebuild package for testing this PR, then your test is not valid as it is not sufficient just to apply the changes e.g. with the patchtester.

Please change back your test result to "Not tested".

richard67 avatar Aug 24 '24 17:08 richard67

@richard67 I do not know how to do that. I don't see a way to edit a previous post. Should I just repost?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34764.

bascherz avatar Aug 24 '24 17:08 bascherz

I have not tested this item.

I do not know what "npm" is or how to execute it. So that aside, after installing the patch I get the following error when either editing an existing article or creating a new one. I have seen this during testing of other patches during PBF.

An error has occurred. 0 There is no "com_content.admin-article-edit" asset of a "script" type in the registry. Call Stack

Function Location

1 () JROOT/libraries/src/WebAsset/WebAssetRegistry.php:135 2 Joomla\CMS\WebAsset\WebAssetRegistry->get() JROOT/libraries/src/WebAsset/WebAssetManager.php:274 3 Joomla\CMS\WebAsset\WebAssetManager->useAsset() JROOT/libraries/src/WebAsset/WebAssetManager.php:208 4 Joomla\CMS\WebAsset\WebAssetManager->__call() JROOT/administrator/components/com_content/tmpl/article/edit.php:26 5 include() JROOT/libraries/src/MVC/View/HtmlView.php:416 6 Joomla\CMS\MVC\View\HtmlView->loadTemplate() JROOT/libraries/src/MVC/View/HtmlView.php:204 7 Joomla\CMS\MVC\View\HtmlView->display() JROOT/administrator/components/com_content/src/View/Article/HtmlView.php:135 8 Joomla\Component\Content\Administrator\View\Article\HtmlView->display() JROOT/libraries/src/MVC/Controller/BaseController.php:697 9 Joomla\CMS\MVC\Controller\BaseController->display() JROOT/administrator/components/com_content/src/Controller/DisplayController.php:65 10 Joomla\Component\Content\Administrator\Controller\DisplayController->display() JROOT/libraries/src/MVC/Controller/BaseController.php:730 11 Joomla\CMS\MVC\Controller\BaseController->execute() JROOT/libraries/src/Dispatcher/ComponentDispatcher.php:143 12 Joomla\CMS\Dispatcher\ComponentDispatcher->dispatch() JROOT/libraries/src/Component/ComponentHelper.php:361 13 Joomla\CMS\Component\ComponentHelper::renderComponent() JROOT/libraries/src/Application/AdministratorApplication.php:150 14 Joomla\CMS\Application\AdministratorApplication->dispatch() JROOT/libraries/src/Application/AdministratorApplication.php:195 15 Joomla\CMS\Application\AdministratorApplication->doExecute() JROOT/libraries/src/Application/CMSApplication.php:306 16 Joomla\CMS\Application\CMSApplication->execute() JROOT/administrator/includes/app.php:58 17 require_once() JROOT/administrator/index.php:32


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34764.

bascherz avatar Aug 24 '24 17:08 bascherz

This pull request has been automatically rebased to 5.3-dev.

HLeithner avatar Sep 02 '24 08:09 HLeithner

This pull request has been automatically rebased to 6.0-dev.

HLeithner avatar Mar 04 '25 17:03 HLeithner

This pull request has been automatically rebased to 6.1-dev.

HLeithner avatar Aug 31 '25 12:08 HLeithner