Skip to content
This repository was archived by the owner on Feb 9, 2019. It is now read-only.

Conversation

@Wang-Yu-Chao
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

New modals for conveniently creating associations

Testing Instructions

Expected result

Actual result

Documentation Changes Required

@Wang-Yu-Chao Wang-Yu-Chao changed the title New ui New modals for conveniently creating associations Jul 18, 2018
@infograf768
Copy link
Contributor

Modal opens Ok with new button Create Associations. 👍
WIP I guess.

var checkedBoxes = document.querySelectorAll("td.row-selected input[type='checkbox']");
var languageIds = [];

checkedBoxes.forEach(function(box) {
Copy link
Contributor

Choose a reason for hiding this comment

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

forEach will not work with NodeList on Microsoft edge and IE.
Replace var checkedBoxes = document.querySelectorAll("td.row-selected input[type='checkbox']");
with
var checkedBoxes = [].slice.call(document.querySelectorAll("td.row-selected input[type='checkbox']"));

Copy link
Contributor

Choose a reason for hiding this comment

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

# Conflicts:
#	administrator/components/com_associations/Helper/AssociationsHelper.php
@infograf768
Copy link
Contributor

infograf768 commented Jul 24, 2018

We have quite a few errors in the modal

[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined property: stdClass::$catid in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 97
[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined property: stdClass::$catid in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 98
[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined property: stdClass::$category in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 102
[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined property: stdClass::$catid in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 103
[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined variable: title in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 123
[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined property: stdClass::$catid in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 97
[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined property: stdClass::$catid in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 98
[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined property: stdClass::$category in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 102
[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined property: stdClass::$catid in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 103
[24-Jul-2018 10:03:47 Europe/Berlin] PHP Notice:  Undefined variable: title in /Applications/MAMP/htdocs/gsoc_multi/administrator/components/com_associations/tmpl/autoassoc/modal.php on line 123

This happens when there is no associated article yet.
It looks like you used a similar code from /administrator/components/com_categories/models/fields/modal/category.php
The big difference here is that we don't want to associate a category, but select/create one for the article item. Therefore $item->catid is wrong.
When there is no associations yet $item only contains
object(stdClass)#379 (5) { ["lang_id"]=> int(2) ["language"]=> string(5) "fr-FR" ["published"]=> int(1) ["language_title"]=> string(11) "French (FR)" ["language_image"]=> string(5) "fr_fr" }

You should look at the modal_category field
example components/com_content/tmpl/category/default.xml

<field 
				name="id"
				type="modal_category"
				label="JGLOBAL_CHOOSE_CATEGORY_LABEL"
				description="JGLOBAL_CHOOSE_CATEGORY_DESC"
				extension="com_content"
				required="true"
				select="true"
				new="true"
				edit="true"
				clear="true"
			/>

The page where such a field displays is for example
screen shot 2018-07-24 at 11 31 40

The main difference is Select category should use $forcedLanguage It is also the case when using Create. In that case the Language field should also be populated by the forcedLanguage.
There is a typo in your code (forcedLangauge instead of forcedLanguage but it is not sufficient to force language. We do have a bug somewhere.

EDIT: in fact we should use there
'&amp;forcedLanguage=' . $item->language;

Another aspect: If we do have already an associated article, we still can try to change its category but we can't save that as anyway there is no tick box.
I suggest that the category Select/Create should not show in this case.
Line 93
Use : <?php if (!empty($this->typeFields['catid']) && !isset($item->item_title)) : ?>

Also:
The code to decide if the item is using categories and therefore propose a Select/Create should also decide about the Category column as it will be totally useless for menu items or Categories association for example.
Use:

							<?php if (!empty($this->typeFields['catid'])) : ?>
								<th class="nowrap text-center">
									<?php echo JText::_('JCATEGORY'); ?>
								</th>
							<?php endif; ?>

The modal should be able to adapt its columns and specific field per item in function of the parameters of the item.
Example:
Menu items should propose choosing a Parent Menu item as well as a specific Menu. Menu should also be a column as in the Menu Items manager.
Categories should propose Parent Category.

It's coming. Courage. :-)

@infograf768
Copy link
Contributor

Forgot:
Looks like new-staging is not up-to-date with 4.0-dev
Recently was solved joomla/joomla-cms#21176
I suggest to update asap

@infograf768
Copy link
Contributor

Conflicts to solve too.

@infograf768
Copy link
Contributor

infograf768 commented Jul 24, 2018

Also found out that <?php echo JHtml::_('jgrid.published', $item->published, $i, 'languages.', true); ?>
can't work as we are not in the Languages Controller.
I think in fact that the icon should never have an onclick. This is not the place to publish or unpublish a content language.
It should be changed to false.

@Wang-Yu-Chao
Copy link
Contributor Author

@infograf768 Many thanks! I've modified the codes according to your comment.

</tr>
</tfoot>
<tbody>
<?php foreach ($this->items as $i => $item) :?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that $this->items is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This loop won't be executed if $this->items is false/null/an empty array. So I think it's ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

$this->items are the Content Languages.
If there are no Content Languages, there is no possible associations and, normally, the component will never display.

@infograf768
Copy link
Contributor

Results of my tests and suggestions:

  1. Close should reload the manager to see the result of the creation.
  2. Ticking the item and THEN Select a category unticks the item. This should be solved.
  3. Edit a Selected category does not work.
  4. BUG: If multiple content languages and only one creation has been done. When reloading the Create Missing Associations modal and trying to create an association for another language (see screenshot of such a situation below) then we get a fatal error:
    Duplicate entry 'com_content.item-1' for key 'PRIMARY'
  5. Create should throw an error if the Category is not selected.

Also:
I think the whole code to Select/Create/Clear/Edit (in our case category) should be totally separated from modal.php. Maybe in a new com_associations specific Layout (using $dispayData for our params) that would be loaded IF we have
<?php if (!empty($this->typeFields['catid'])) : ?>

The reason is that we also want to be able to Create associations for other type of items than components using categories.

As I wrote earlier, we may want to choose Parent Category, Parent Menu item, etc. and we would also have a conditional to display the parameter to choose via a new layout in the parameter column.

Therefore, I do not think that we should add the code to define a category in the category column.

I propose this kind of layout:
Here an association was already created. Another one may need to be created (this to show both situations).
screen shot 2018-07-25 at 12 04 29

As you can see I totally took off the category column as it displays in the Parameter column which would be used for all cases.

@infograf768
Copy link
Contributor

infograf768 commented Jul 25, 2018

BTW, when this will be completed, I have a patch ready to get better titles for the Modal.

@infograf768
Copy link
Contributor

UI good 👍

Tests

  1. I found some JFactory while we have use Joomla\CMS\Factory; in a few files.
  2. Even after correcting this, I have a weird error after this patch, but NOT only in com_associations. The error happens also when I edit an article via the normal Articles manager
    Class 'Joomla\Component\Content\Administrator\Helper\Factory' not found

screen shot 2018-07-29 at 09 25 27

@Wang-Yu-Chao
Copy link
Contributor Author

That's strange... I couldn't reproduce that error on my computer. Articles manager works in the right way.
image

@Wang-Yu-Chao
Copy link
Contributor Author

About refreshing the page after closing modals, I'm not sure how to change jQuery('.modal').on('hidden.bs.modal', ...); to es6, which is used in admin-menus-default.js.

@infograf768
Copy link
Contributor

It does not exist yet as es6, but are you sure this is the place where the changes should occur and not in admin-autoassoc-modal ?

@Wang-Yu-Chao
Copy link
Contributor Author

I did not explain clearly. 😛This kind of code is used in admin-menus-default.js. And it's perfectly fit for admin-associations-default.js. Once the hidden.bs.modal event is raised, the MA manager page will be refreshed.

@infograf768
Copy link
Contributor

BTW, I solved here the undefined property $featured with this code

				// Get the featured state for articles
				if ($extensionName == 'com_content')
				{
					$featured = $table->featured;
				}

				// Check if the article was featured and update the #__content_frontpage table
				if ($extensionName == 'com_content' && $featured == 1)
				{
					$db = $this->getDbo();
					$query = $db->getQuery(true)
						->insert($db->quoteName('#__content_frontpage'))
						->values($newId . ', 0');
					$db->setQuery($query);
					$db->execute();
				}

@infograf768
Copy link
Contributor

@Wang-Yu-Chao
Did not really check why you modify multiselect.js, but beware: Since a few minutes ago it is now an es6 file.

@infograf768
Copy link
Contributor

and a lot of other changes in 4.0-dev ;)

@Wang-Yu-Chao
Copy link
Contributor Author

Oh thanks @infograf768! I should have found that problem.

There was a bug in multiselect.js. Seems that it was solved in the latest updates. So no bother 😄

@Wang-Yu-Chao
Copy link
Contributor Author

Added a new parameter "Parent Item" for menus.
New UI looks like this:
image

@infograf768
Copy link
Contributor

Hmm
Broken here with these errors:
screen shot 2018-08-07 at 17 12 39

@dgrammatiko
Copy link
Contributor

@dgrammatiko
Copy link
Contributor

@infograf768 do you have the PR for the tinyMCE? If not please resync this to the 4.0-dev

@infograf768
Copy link
Contributor

@dgrammatiko
I am using an updated instance of the branch towards which this PR is made, i.e. new-staging, not 4.0-dev.

If I simply add

        if (!this.boxes.length) {
            return;
          }

and then run npm install
the resulting multiselect.js does not contain the code
I get

value: function onRowClick(event) {
        var currentRowNum = this.rows.indexOf(event.target.closest('tr'));
        var currentCheckBox = this.checkallToggle ? currentRowNum + 1 : currentRowNum;
        var isChecked = this.boxes[currentCheckBox].checked;

        if (currentCheckBox >= 0) {
etc.

@dgrammatiko
Copy link
Contributor

does this branch has the multiselect.es6.js or the old one?

Please resync the repo, this is already patched

@dgrammatiko
Copy link
Contributor

Are you testing a staging branch here, not the 4.0-dev?

@infograf768
Copy link
Contributor

@dgrammatiko

I am using the specific GSOC branch called joomla-projects:new-staging towards which this PR is made.
It is not in sync with present 4.0-dev.

@infograf768
Copy link
Contributor

It contains a multiselect.es6.js which I modofied with the code you gave above.

screen shot 2018-08-07 at 18 01 55

@dgrammatiko
Copy link
Contributor

dgrammatiko commented Aug 7, 2018 via email

@infograf768
Copy link
Contributor

Looks like the file is very different in that new-staging branch... thus why the code added was not line 52.

Also, this patch contains an extraneous admin-autoassoc-modal.js in build/media which I now take off as we have an es6 one in media_src

I copied the build from last 4.0-dev and ran npm install

@Wang-Yu-Chao
I do not have any more the multiselect error from the beginning, but an error in the admin-autoassoc-modal.js created in media:
document is undefined admin-autoassoc-modal.js:51:3

the line is
document.addEventListener('DOMContentLoaded', onBoot, true);

I changed some stuff there
used
(function (document, Joomla) {
instead of
(function (Joomla, document) {

and ended the js by
})(document, Joomla);
instead of
window.Joomla

(Changes to be done in the es.6 file)

Now the modal works when creating an association for ONE language.
If I try to create an association for another language I have again an error:
TypeError: this.boxes[currentCheckBox] is undefined multiselect.js:84:13
and a
Save failed with the following error: No item selected.

In any case, choosing a parent item does not work at all. Is missing in the list Menu Item Root and, alas, after choosing a Menu, the list is not filled with that specific menu items.

@dgrammatiko
Even after using dev-4.0 build I do not see the lines

if (!this.boxes.length) {
        return;
      }

in the media multiselect.js here.

@dgrammatiko
Copy link
Contributor

(function (Joomla, document) {

there is no ES5 in 4.-dev. I guess you've messed up things here. The code here is for J3 and you try to run in J4...

@infograf768
Copy link
Contributor

infograf768 commented Aug 8, 2018

Yeah, in fact it is sufficient for the admin-autoassoc-modal.es6.js to change the last line to
})(Joomla, document); instead of window.document

It is AFTER running npm install that is also created in media/ the file admin-autoassoc-modal.js
which contains (function (Joomla, document) {

@infograf768
Copy link
Contributor

@Wang-Yu-Chao
Sorry, but 4.0-dev has still changed a lot!
We have now in core a new workflow component and new tables.
This means new-staging has to be updated, your branch too, and patch also.

BTW for the new menu stuff I modified here a bit to get "No Parent item" at least.

				$field = $fieldset->addChild('field');
				$field->addAttribute('name', 'MenuType_' . $language->lang_id);
				$field->addAttribute('type', 'menu');
				$field->addAttribute('label', 'COM_MENUS_ITEM_FIELD_ASSIGNED_LABEL');
				$field->addAttribute('clientid', '0');

				$field = $fieldset->addChild('field');
				$field->addAttribute('name', 'MenuParent_' . $language->lang_id);
				$field->addAttribute('type', 'menuparent');
				$field->addAttribute('label', 'COM_MENUS_ITEM_FIELD_PARENT_LABEL');
				$field->addAttribute('clientid', '0');
				$option = $field->addChild('option', 'COM_MENUS_ITEM_ROOT');
				$option->addAttribute('value', '1');

But it is still quite hard to choose a parent when the list is not filtered by Menu, including all menu items set to the desired language or ALL.
If it had been only for the desired language we could have used module_menu instead of parent type, but we need also items set to ALL.

@Wang-Yu-Chao
Copy link
Contributor Author

@infograf768 Sorry for the late reply. I've been traveling around in the summer vacation. Will update this code recently.

@infograf768
Copy link
Contributor

No p @Wang-Yu-Chao

hard to go on working with all the changes we have with workflow and the necessitiy of using npm/node to test stuff.

@infograf768
Copy link
Contributor

infograf768 commented Aug 27, 2018

IMHO, we need new modals for menus (Parent items) and categories to choose stuff:
They should present items tagged to ALL languages as well as to the forcedlanguage + a ROOT.
The fields we have are too unclear.

zero-24 pushed a commit that referenced this pull request Sep 6, 2018
* Load correct core files of override files (#2)

Start implements loadcorefile() in administrator/components/com_templates/Model/TemplateModel.php

* CS (#3) Coding Standards

* codingstandards

* codingstandards (#4)

* Test (#6)

Phase 2 (2 part) Mechanism to find correct core file and implementation.

* Remove Notice: Only available for html-folder

* Remove Warning if core file not found (#11)

Thanks.
So one part of the issue joomla-projects/gsoc18_override_management#12 is done.

* Implement the diff view in template manager 

Implement the diff view in template manager

* coding standard (#17)

* fix diff (#18) Fix bug in path in case of administrator template override.

Fix bug in path in case of administrator template override.

* Notification after update and TEST (#16)

Find changed files of overridden files and show message.

* coding standard (#21)

* correction

* correction (#26)

* Correcthtmlpath (#27)

* correction

* change oldhtml to newhtml

* List of updated override files. (#30)

* addcss (#34)

* Final Product  (#39)

Core and Diff view
Updated override history list.
Quick icon notification plugin.
Override control plugin.

* save 3 lines :)

* New feature show status. (#47)

show status in com_template view templates

* link

* corrected namespace

* Button to Switch (#35)

* wip add Switcher

* wip style switcher

* wip style switch make inline and change on off text

* wip start with js

* wip js

* wip delete buttons and make js more robust

* wip save to storage

* wip delete old code

* wip

* wip lint

* wip css

* set default value for switcher

* wip make switcher blue

* wip

* wip

* build

* correct names

* create new functions

* fist test code

* use onchange

* undo installer.min.js

* add forgotten new line at the end of css file

* correct align

* correct compare.es6 - only deleted the toggle part

* correct compare.js - only deleted the toggle part

* wip

* reduce timeout

* wrap in funcitons

* wip

* add use strict to both js-files(compare and toggle)

* add the timeout value of 500 again, because 200 are not enought in my case

* use css class 'active' for toggle views

* add strict

* time out for editor

* wip

* improvments use newActive and switch

* correction

* width of switcher-spans

* correct align

* do not use global

* wip

* removed timeouts

* JTEXT to TEXT

* forgotton last line

* deleted duplicated comments

* css fix align

* use unnamed functions in es6

* Sql files for fix database (#50)

* sql files for database fix

* delete space

* Suggestion for displaying Dates in view updates files (#52)

Correct Dates and do not use date of file any more

* Store Date as UTC and show it in server time zone (#57)

* modified and created date are created and stored in UTC

* convert dates for displaying in model

* spar a loop

* normalize timezone in view

* use language constants for dateformat

* JToolbarHelper to ToolbarHelper

* CS

* namespace

* plural

* name

* clean

* text

* fx

* sin

* files

* s

* Suggestion for language strings (#60)

* language strings

* correct typo

* delete media folder plg_quickicon

* add folder plg_quickicon to build/media_src

* delete files in media folder

* Move media folder - System (#66)

* multi

* cs

* delete files in media folder for joomla toolbar (#67)

* Fix button switchers style. (#70)

* button

* CS

* changed uitab.addTab for updated files

* Bring back core.js changes. (#69)

* core.js

* const

* fix

* form

* core

* hound

* CS

* scopr

* grid

* alpha

* cs

* lang

* only override file

* lang

* override lang installer

* Cs

* sub

* Update list of core extensions (#71)

* Language changes (#76)

* update

* Update en-GB.com_templates.ini

* override JLIB_HTML_PUBLISH_ITEM

this is the hover text on the publish icon in the list of files

* Change icon (#74)

change the icon to use an outline for more consistency

* lang

* not core (#75)

* not core

* Update en-GB.plg_installer_override.ini

* namespace

* cs

* Updated files (#82)

* Update default_updated_files.php

* Update en-GB.com_templates.ini

* Update en-GB.com_templates.ini (#81)

* Update en-GB.plg_quickicon_overridecheck.ini (#80)

* Update en-GB.plg_quickicon_overridecheck.ini (#79)

* remove space (#78)

* Update en-GB.plg_quickicon_overridecheck.ini

* Update en-GB.plg_quickicon_overridecheck.sys.ini

* remove hardcoded id

* null get function

* state

* clean

* More changes "core" to "original" (#85)

* cs

* update

* plural
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants