NPM Resource Changed bug Maintainers Checked PR-5.1-dev Pending

User tests: Successful: Unsuccessful:

avatar rajputanuj31
rajputanuj31
8 Feb 2023

Pull Request for issue #33637
Summary of Changes
Multi select without using shift key

Testing Instructions

Open Media
Try to Select media manager files.

Actual result BEFORE applying this Pull Request

You can not select multiple items without holding the shift key.

Expected result AFTER applying this Pull Request

You can select multiple items by holding ctrl key & clicking on items.
You can also select all files between two selected files by holding shift key.

Media.-Trim-2.mp4
avatar joomla-cms-bot joomla-cms-bot - change - 8 Feb 2023
Category JavaScript Administration com_media NPM Change
avatar rajputanuj31 rajputanuj31 - open - 8 Feb 2023
avatar rajputanuj31 rajputanuj31 - change - 8 Feb 2023
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 8 Feb 2023
Category JavaScript Administration com_media NPM Change JavaScript Administration com_media NPM Change com_menus Libraries Modules Front End
avatar rajputanuj31 rajputanuj31 - change - 8 Feb 2023
Labels Added: NPM Resource Changed PR-4.3-dev
avatar richard67
richard67 - comment - 8 Feb 2023

@rajputanuj31 When you create a PR, why do you delete stuff which is prepared in the description, like e.g. the "Pull request for issue #..." at the top, or the section about documentation, which requires the right check boxes to be checked? We do not have that stuff for nothing in our pull request description template. So please add the Pull Request for issue #33637 to the top so we know for which issue the pull request is made, and add back the documentation section and check the right check boxes. Thanks in advance.

avatar rajputanuj31 rajputanuj31 - change - 8 Feb 2023
The description was changed
avatar rajputanuj31 rajputanuj31 - edited - 8 Feb 2023
avatar brianteeman
brianteeman - comment - 8 Feb 2023

There are unrelated files in this pull request :(

avatar joomla-cms-bot joomla-cms-bot - change - 8 Feb 2023
Category JavaScript Administration com_media NPM Change com_menus Libraries Modules Front End JavaScript Administration com_media NPM Change
avatar rajputanuj31
rajputanuj31 - comment - 8 Feb 2023

There are unrelated files in this pull request :(

Fixed.

avatar rajputanuj31 rajputanuj31 - change - 9 Feb 2023
The description was changed
avatar rajputanuj31 rajputanuj31 - edited - 9 Feb 2023
avatar dgrammatiko
dgrammatiko - comment - 9 Feb 2023

@rajputanuj31 can you check if the rtl languages need any adjustment? Other than that this seems ready for some testing

avatar rajputanuj31
rajputanuj31 - comment - 9 Feb 2023

@rajputanuj31 can you check if the rtl languages need any adjustment? Other than that this seems ready for some testing

it's working fine.
@dgrammatiko could you please test this PR.

avatar laoneo
laoneo - comment - 10 Feb 2023

Please add some inline comments, so we can follow your code. Actually it is hard to understand what exactly is going on with all these if else conditions. I would also recommend that you do early return instead of so many else statements.

avatar brianteeman
brianteeman - comment - 10 Feb 2023

I have tested this and it works perfectly apart from on the first page which is very weird.

In this video I am shift clicking on the last image and then on the image before it and you will see that a third image is also selected. I can't replicate that on any other screen.

shift

shift2

avatar rajputanuj31
rajputanuj31 - comment - 10 Feb 2023

I have tested this and it works perfectly apart from on the first page which is very weird.

In this video I am shift clicking on the last image and then on the image before it and you will see that a third image is also selected. I can't replicate that on any other screen.

@brianteeman It's working perfectly in my local machine. what should I do now?
@dgrammatiko @laoneo can you please confirm this issue.

avatar brianteeman
brianteeman - comment - 10 Feb 2023

do you have exactly the same images as I have?

avatar rajputanuj31
rajputanuj31 - comment - 10 Feb 2023

do you have exactly the same images as I have?

Yes I do have the same images.

avatar brianteeman
brianteeman - comment - 10 Feb 2023

do you have exactly the same images as I have?

Yes I do have the same images.

all three?

avatar rajputanuj31
rajputanuj31 - comment - 10 Feb 2023

do you have exactly the same images as I have?

Yes I do have the same images.

all three?

Yes now I am also facing the same issue. I am trying to debug it.

avatar rajputanuj31
rajputanuj31 - comment - 10 Feb 2023

@brianteeman @dgrammatiko @laoneo I found the reason for this behavior.
In Joomla media items storing index is different from index of item's being shown on browser section.
When an image or dir. name is starting with capital letter then it's storing index in array is different from what it's being displayed.
image
Carefully observe the index of Photo.jpg is stored as index of 4 but it's being displayed as if it is 5th index.
How do I get the Index's of the items being viewed in browser????

avatar dgrammatiko
dgrammatiko - comment - 10 Feb 2023

How do I get the Index's of the items being viewed in browser????

I intentionally renamed the items to localItems to make it obvious that the items in the browser component are localised, eg reordered. In short use the localItems:

avatar rajputanuj31
rajputanuj31 - comment - 10 Feb 2023

@brianteeman @dgrammatiko @laoneo I have made some changes Please review it.

avatar brianteeman brianteeman - test_item - 10 Feb 2023 - Tested successfully
avatar brianteeman
brianteeman - comment - 10 Feb 2023

I have tested this item successfully on 005b0d6


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

avatar dgrammatiko
dgrammatiko - comment - 10 Feb 2023

@rajputanuj31 could you do a small refactoring here since we have the same code in 2 files.

Create a new file administrator/components/com_media/resources/scripts/components/browser/utils/utils.es6.js

with contents:
export default {
  /**
   * Handle the click event
   * @param event
   * @param ctx  the context
   */
  onItemClick: (event, ctx) => {
    if (ctx.item.path && ctx.item.type === 'file') {
      window.parent.document.dispatchEvent(
        new CustomEvent('onMediaFileSelected', {
          bubbles: true,
          cancelable: false,
          detail: {
            path: ctx.item.path,
            thumb: ctx.item.thumb,
            fileType: ctx.item.mime_type ? ctx.item.mime_type : false,
            extension: ctx.item.extension ? ctx.item.extension : false,
            width: ctx.item.width ? ctx.item.width : 0,
            height: ctx.item.height ? ctx.item.height : 0,
          },
        }),
      );
    }

    if (ctx.item.type === 'dir') {
      window.parent.document.dispatchEvent(
        new CustomEvent('onMediaFileSelected', {
          bubbles: true,
          cancelable: false,
          detail: {},
        }),
      );
    }

    // Handle clicks when the item was not selected
    if (!ctx.isSelected()) {
      // Handle clicks when shift key was pressed
      if (event.shiftKey || event.keyCode === 13) {
        const currentIndex = ctx.localItems.indexOf(ctx.$store.state.selectedItems[0]);
        const endindex = ctx.localItems.indexOf(ctx.item);
        // Handle selections from up to down
        if (currentIndex < endindex) {
          ctx.localItems.slice(currentIndex, endindex + 1)
            .forEach((element) => ctx.$store.commit(types.SELECT_BROWSER_ITEM, element));
        // Handle selections from down to up
        } else {
          ctx.localItems.slice(endindex, currentIndex)
            .forEach((element) => ctx.$store.commit(types.SELECT_BROWSER_ITEM, element));
        }
        // Handle clicks when ctrl key was pressed
      } else if (event[/Mac|Mac OS|MacIntel/gi.test(window.navigator.userAgent) ? 'metaKey ' : 'ctrlKey'] || event.keyCode === 17) {
        ctx.$store.commit(types.SELECT_BROWSER_ITEM, ctx.item);
      } else {
        ctx.$store.commit(types.UNSELECT_ALL_BROWSER_ITEMS);
        ctx.$store.commit(types.SELECT_BROWSER_ITEM, ctx.item);
      }
      return;
    }
    ctx.$store.dispatch('toggleBrowserItemSelect', ctx.item);
    window.parent.document.dispatchEvent(
      new CustomEvent('onMediaFileSelected', {
        bubbles: true,
        cancelable: false,
        detail: {},
      }),
    );

    // If more than one item was selected and the user clicks again on the selected item,
    // he most probably wants to unselect all other items.
    if (ctx.$store.state.selectedItems.length > 1) {
      ctx.$store.commit(types.UNSELECT_ALL_BROWSER_ITEMS);
      ctx.$store.commit(types.SELECT_BROWSER_ITEM, ctx.item);
    }
  },
}

Then change the handleClick method in the item https://github.com/joomla/joomla-cms/pull/39824/files#diff-68de052c5527b63cd4fa16f4872cdeca97b20ac05389de587af9c8b14a698d50R126-R193 and 'onClick' in the row https://github.com/joomla/joomla-cms/pull/39824/files#diff-e3f0552dc23736cd5bd70c04dab063ad0c26e284be8542e0dad16b04d4c5da21R104-R162 to

    handleClick(event, item) {
      return onItemClick(event, item);
    },

and

    onClick(event, item) {
      return onItemClick(event, item);
    },

respectively. You need to import the onItemClick as usual: import { onItemClick } from '../utils/utils.es6.js';

avatar rajputanuj31
rajputanuj31 - comment - 10 Feb 2023

@dgrammatiko Please have a look.

avatar rajputanuj31
rajputanuj31 - comment - 10 Feb 2023

@dgrammatiko I have made all changes as you suggest. Is everything looks fine??

avatar dgrammatiko
dgrammatiko - comment - 10 Feb 2023

Is everything looks fine??

Nope, the build is broken, check the npm part on the CI: https://ci.joomla.org/joomla/joomla-cms/61729

avatar rajputanuj31
rajputanuj31 - comment - 10 Feb 2023

@brianteeman @dgrammatiko Please test it. On my local machine it's working perfectly.

avatar rajputanuj31
rajputanuj31 - comment - 11 Feb 2023

@dgrammatiko Please have a look. Is now everything looks correct?

avatar dgrammatiko
dgrammatiko - comment - 16 Feb 2023

@laoneo could you also review this one?

avatar rajputanuj31
rajputanuj31 - comment - 17 Feb 2023

@dgrammatiko @laoneo @brianteeman please test the PR.

avatar rajputanuj31
rajputanuj31 - comment - 15 Jun 2023

Hey @laoneo @dgrammatiko @brianteeman @Hackwar I think this PR is ready so please test it. If PR is not ready please leave a comment.

avatar ceford ceford - test_item - 15 Sep 2023 - Tested unsuccessfully
avatar ceford
ceford - comment - 15 Sep 2023

I have tested this item ? unsuccessfully on f971327

There is something wrong here. I applied the patch and ran npm ci (and even npm install). I am using Firefox on a Mac with 5.0.0-beta2-dev and I see no difference in the behaviour with the patch installed and hard page reload. The Ctrl key has no effect. I checked the code to see that all the patches were applied. On Mac/Firefox the Ctrl key brings up a browser dialog (Back, Reload, etc). I could not find a combination of keys that select/deselect an image that does not deselect previously selected images.


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

avatar HLeithner
HLeithner - comment - 5 Oct 2023

@rajputanuj31 can you look at the response by @ceford please?

avatar rajputanuj31
rajputanuj31 - comment - 5 Oct 2023

@rajputanuj31 can you look at the response by @ceford please?

Its working perfectly in my local machine.
Screencast from 05-10-23 06:23:07 PM IST.webm

avatar dgrammatiko
dgrammatiko - comment - 5 Oct 2023

Mac ctrl (in this context) is the apple button (command)

avatar richard67 richard67 - change - 23 Oct 2023
Title
[4.3]Corrected different multi-select behavior in media manager.
[5.0]Corrected different multi-select behavior in media manager.
avatar richard67 richard67 - edited - 23 Oct 2023
avatar richard67 richard67 - change - 23 Oct 2023
Title
[5.0]Corrected different multi-select behavior in media manager.
[5.0] Corrected different multi-select behavior in media manager.
avatar richard67 richard67 - edited - 23 Oct 2023
avatar HLeithner HLeithner - change - 24 Apr 2024
Title
[5.0] Corrected different multi-select behavior in media manager.
[5.1] Corrected different multi-select behavior in media manager.
avatar HLeithner HLeithner - edited - 24 Apr 2024
avatar webmasterab
webmasterab - comment - 9 Jun 2024

I tested it and it didn't pass.
See video.
Windows PC
Chrome latest version
CTRL pressed

patchtest.mp4
avatar fgsw
fgsw - comment - 29 Aug 2024

No prebuilt packages are available to test.

avatar Hackwar Hackwar - change - 29 Aug 2024
Labels Added: bug Maintainers Checked PR-5.1-dev
Removed: PR-4.3-dev
avatar Hackwar
Hackwar - comment - 29 Aug 2024

In about 10 minutes the packages should be available again.

avatar fgsw fgsw - test_item - 29 Aug 2024 - Tested successfully
avatar fgsw
fgsw - comment - 29 Aug 2024

I have tested this item ✅ successfully on b86e7d4

Comments:

  • Select multiple items by holding command key
  • Select all files between two selected files by holding shift key

Test System Information:

PHP Built On Darwin Air.local 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:13:00 PDT 2024; root:xnu-10063.141.2~1/RELEASE_X86_64 x86_64
Database Type mysql
Database Version 8.0.35
Database Collation utf8mb4_unicode_ci
Database Connection Collation utf8mb4_0900_ai_ci
Database Connection Encryption None
Database Server Supports Connection Encryption Yes
PHP Version 8.3.8
Web Server Apache/2.4.58 (Unix) OpenSSL/1.1.1u mod_fastcgi/mod_fastcgi-SNAP-0910052141
WebServer to PHP Interface cgi-fcgi
Joomla! Version Joomla! ‎5.1.5-dev+pr.39824
Joomla Backward Compatibility Plugin Disabled
User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:129.0) Gecko/20100101 Firefox/129.0


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

avatar HLeithner
HLeithner - comment - 2 Sep 2024

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

avatar dgrammatiko
dgrammatiko - comment - 2 Sep 2024

Note for maintainers: this PR needs update so it won't revert #43859

basically adding name: ctx.item.name, should be enough after line 15 in
administrator/components/com_media/resources/scripts/components/browser/utils/utils.es6.js

avatar HLeithner HLeithner - change - 2 Sep 2024
Title
[5.1] Corrected different multi-select behavior in media manager.
[5.2] Corrected different multi-select behavior in media manager.
avatar HLeithner HLeithner - edited - 2 Sep 2024

Add a Comment

Login with GitHub to post a comment