Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Non-clickable module in a menu item #23609

Closed
wants to merge 21 commits into from

Conversation

amitranjan2
Copy link
Contributor

Pull Request for Issue #23540

Summary of Changes

Added data-toggle and data-target

Testing Instructions

Navigate to following page from the admin page
Menus >> Main Menu >> Home

Open module assignment tab

try to click item

Expected result

Modal open to edit

Actual result

Modal not opening

Documentation Changes Required

No

@amitranjan2
Copy link
Contributor Author

@dgrammatiko Please review.

@infograf768
Copy link
Member

infograf768 commented Jan 21, 2019

Partially successful.
Although we now get the modal, Saving (or Save and Close) does not save the modifications of the module.

TypeError: c.contents is not a function admin-item-edit_modules.min.js:1:1867

debug on
TypeError: iframe.contents is not a function admin-item-edit_modules.js:89:13

@amitranjan2
Copy link
Contributor Author

ok thanks, I will fix

@mvanvu
Copy link
Contributor

mvanvu commented Jan 21, 2019

/build/media_src/com_menus/js/admin-item-edit_modules.es6.js line 75
iFrame.setAttribute('class', 'class="iframe jviewport-height70"'); is that right?

OR this should be iFrame.setAttribute('class', 'iframe jviewport-height70'); ?

@dgrammatiko
Copy link
Contributor

dgrammatiko commented Jan 21, 2019

@amitranjan2 can you please use the event listener? There was a reason I did it like that in the first place: https://github.com/joomla/joomla-cms/blob/4.0-dev/build/media/legacy/js/bootstrap-init.js#L97

So your code essentially will never work...

@dgrammatiko
Copy link
Contributor

Although we now get the modal, Saving (or Save and Close) does not save the modifications of the module.

@infograf768 please test:
#23600

@amitranjan2
Copy link
Contributor Author

`

@amitranjan2 can you please use the event listener? There was a reason I did it like that in the first place: https://github.com/joomla/joomla-cms/blob/4.0-dev/build/media/legacy/js/bootstrap-init.js#L97

So your code essentially will never work...`

Yes i tried with modal.addEventListener('show.bs.modal', (ev) => {} but its not executing when the modal is open.
Can you please give me example?
@dgrammatiko

@dgrammatiko
Copy link
Contributor

dgrammatiko commented Jan 22, 2019

@amitranjan2 here is some code, admin-item-edit_modules.es6.js:

/**
 * @copyright  Copyright (C) 2005 - 2018 Open Source Matters, Inc. All rights reserved.
 * @license    GNU General Public License version 2 or later; see LICENSE.txt
 */

Joomla = window.Joomla || {};

(() => {
  'use strict';

  const options = Joomla.getOptions('menus-edit-modules');

  if (options) {
    window.viewLevels = options.viewLevels;
    window.menuId = parseInt(options.itemId, 10);
  }

  document.addEventListener('DOMContentLoaded', () => {
    const baseLink = 'index.php?option=com_modules&client_id=0&task=module.edit&tmpl=component&view=module&layout=modal&id=';
    const assigned1 = document.getElementById('jform_toggle_modules_assigned1');
    const assigned0 = document.getElementById('jform_toggle_modules_assigned0');
    const published1 = document.getElementById('jform_toggle_modules_published1');
    const published0 = document.getElementById('jform_toggle_modules_published0');
    const linkElements = [].slice.call(document.getElementsByClassName('module-edit-link'));
    const elements = [].slice.call(document.querySelectorAll('#moduleEditModal .modal-footer .btn'));

    if (assigned1) {
      assigned1.addEventListener('click', () => {
        const list = [].slice.call(document.querySelectorAll('tr.no'));

        list.forEach((item) => {
          item.style.display = 'table-row';
        });
      });
    }

    if (assigned0) {
      assigned0.addEventListener('click', () => {
        const list = [].slice.call(document.querySelectorAll('tr.no'));

        list.forEach((item) => {
          item.style.display = 'none';
        });
      });
    }

    if (published1) {
      published1.addEventListener('click', () => {
        const list = [].slice.call(document.querySelectorAll('.table tr.unpublished'));

        list.forEach((item) => {
          item.style.display = 'table-row';
        });
      });
    }

    if (published0) {
      published0.addEventListener('click', () => {
        const list = [].slice.call(document.querySelectorAll('.table tr.unpublished'));

        list.forEach((item) => {
          item.style.display = 'none';
        });
      });
    }

    if (linkElements.length) {
      linkElements.forEach((linkElement) => {
        linkElement.addEventListener('click', (event) => {
          debugger;
          const link = baseLink + event.target.getAttribute('data-module-id');
          const modal = document.getElementById('moduleEditModal');
          const body = modal.querySelector('.modal-body');
          const iFrame = document.createElement('iframe');

          iFrame.src = link;
          iFrame.setAttribute('class', 'class="iframe jviewport-height70"');
          body.innerHTML = '';
          body.appendChild(iFrame);

          modal.open();
        });
      });
    }

    if (elements.length) {
      elements.forEach((element) => {
        element.addEventListener('click', (event) => {
          const target = event.target.getAttribute('data-target');

          if (target) {
            const iframe = document.querySelector('#moduleEditModal iframe');
            const iframeDocument = iframe.contentDocument || iframe.contentWindow.document;
            iframeDocument.querySelector(target).click();
          }
        });
      });
    }
  });
})();

Also admin-module-edit.es6.js line 18:

          const updPosition = document.getElementById('jform_position').value;

Also please remove all the changes in the markup:

					data-toggle="modal"
						data-target="#moduleEditModal"

This is coupling things to Bootstrap and we don't want that!!!

@amitranjan2
Copy link
Contributor Author

thanks, I will update.
@dgrammatiko

@amitranjan2
Copy link
Contributor Author

please test this @Quy

@amitranjan2
Copy link
Contributor Author

@infograf768 Please review this
I think now it's working fine.
opening as well as saving both, working fine

@ghost ghost added the J4 Issue label Apr 5, 2019
@ghost ghost removed the J4 Issue label Apr 13, 2019
@ghost ghost changed the title [Fix] [4.0] Non-clickable module in a menu item [4.0] Non-clickable module in a menu item Apr 19, 2019
@joomla-cms-bot joomla-cms-bot added the NPM Resource Changed This Pull Request can't be tested by Patchtester label Apr 19, 2019
@ghost ghost added the Conflicting Files label Jun 21, 2019
@ghost
Copy link

ghost commented Sep 2, 2019

Please resolve conflicting files so this PR can get tested at Worldwide Pizza, Bugs & Fun, October 19th

@ttplpoojak
Copy link

I have tested this item ✅ successfully on 64e2b30


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

1 similar comment
@Nilesh5995
Copy link

I have tested this item ✅ successfully on 64e2b30


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

@alikon
Copy link
Contributor

alikon commented Nov 2, 2019

@amitranjan2 can you solve conflict please

@KishoriBKarale
Copy link

I have tested this item ✅ successfully on 64e2b30


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

@pdavila2709
Copy link

I can not apply the patch with the Joomla Patch Tester. This is the message:

Error: The file marked for modification does not exist: build/media_src/com_menus/js/admin-item-edit_modules.es6.js


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

@infograf768
Copy link
Member

@amitranjan2 please solve conflicts

brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Jan 9, 2020
This is a replacement PR for joomla#23609 which resolves the merge conflicts

To test
npm i

Go the module assignment tab of any menu item
Click on the name of a module
A modal will open
Change the published state of the modal and save and close
Check that the module is now shown with the changed state
Check that there are no js errors in the console
@Quy Quy changed the title [4.0] Non-clickable module in a menu item [4.0] Non-clickable module in a menu item Jan 9, 2020
@Quy Quy closed this Jan 9, 2020
@Quy
Copy link
Contributor

Quy commented Jan 9, 2020

Replaced with PR #27455


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

infograf768 added a commit that referenced this pull request Jan 12, 2020
* [4.0] Non-clickable module in a menu item

This is a replacement PR for #23609 which resolves the merge conflicts

To test
npm i

Go the module assignment tab of any menu item
Click on the name of a module
A modal will open
Change the published state of the modal and save and close
Check that the module is now shown with the changed state
Check that there are no js errors in the console

* cs

Co-authored-by: infograf768 <infografjms@gmail.com>
brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Feb 4, 2020
* [4.0] Non-clickable module in a menu item

This is a replacement PR for joomla#23609 which resolves the merge conflicts

To test
npm i

Go the module assignment tab of any menu item
Click on the name of a module
A modal will open
Change the published state of the modal and save and close
Check that the module is now shown with the changed state
Check that there are no js errors in the console

* cs

Co-authored-by: infograf768 <infografjms@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Conflicting Files NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet