[5.1] Corrected different multi-select behavior in media manager.

[rajputanuj31]

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.

https://user-images.githubusercontent.com/115709571/217732378-40a4b298-96dd-4d96-a551-c80627af4c73.mp4

[richard67]

@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.

[brianteeman]

There are unrelated files in this pull request :(

[rajputanuj31]

There are unrelated files in this pull request :(

Fixed.

[dgrammatiko]

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

[rajputanuj31]

@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.

[laoneo]

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.

[brianteeman]

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.

[rajputanuj31]

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.

[brianteeman]

do you have exactly the same images as I have?

[rajputanuj31]

do you have exactly the same images as I have?

Yes I do have the same images.

[brianteeman]

do you have exactly the same images as I have?

Yes I do have the same images.

all three?

[rajputanuj31]

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.

[rajputanuj31]

@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. 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????

[dgrammatiko]

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: https://github.com/joomla/joomla-cms/blob/7f7e2324b62562b8872142cc68d6422a30a2dd62/administrator/components/com_media/resources/scripts/components/browser/browser.vue#L112

[rajputanuj31]

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

[brianteeman]

I have tested this item :white_check_mark: successfully on 005b0d66c22d32be522081167351a9fe190f717e

---

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

[dgrammatiko]

@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';

[rajputanuj31]

@dgrammatiko Please have a look.

[rajputanuj31]

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

[dgrammatiko]

Is everything looks fine??

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

[rajputanuj31]

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

[rajputanuj31]

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

[dgrammatiko]

@laoneo could you also review this one?

[rajputanuj31]

@dgrammatiko @laoneo @brianteeman please test the PR.

[rajputanuj31]

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

[ceford]

I have tested this item :red_circle: unsuccessfully on f9713277467ede118677915a0a3270bf1d0dc3d2

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.}

[HLeithner]

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

[rajputanuj31]

@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

[dgrammatiko]

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

[webmasterab]

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

https://github.com/joomla/joomla-cms/assets/8011751/7ebbc3c2-bc19-4bd7-b227-701bb4ae0697