Skip to content

Conversation

@C-Lodder
Copy link
Member

Pull Request for Issue #41309

Summary of Changes

  1. Fix the treeselect dropdowns not working
  2. Fix the check/uncheck buttons not toggling checkboxes that had been manually changed (backported from [5.1] Rewrite treeselect in vanilla JS #42776)
  3. Fix select/deselect changing the state of the current menu (backported from [5.1] Rewrite treeselect in vanilla JS #42776)

Testing Instructions

  1. Go to edit any module in the Module Manager
  2. Navigate to the Menu Assignment tab
  3. Change the assignment to "Only on selected pages"

Test 1:
Ensure the dropdown menu opens as shown below:
image

Test 2:

  1. Open the dropdown menu and click "select" or "unselect"
    All checkboxes (including the one parallel to the dropdown toggle) should be toggled

Test 3:

  1. Check (or uncheck) any checkbox
  2. Click the buttons shown above:
    image

All checkboxes should be toggled.

@LadySolveig @Quy

@Quy
Copy link
Contributor

Quy commented Feb 26, 2024

Can this be improved to not toggle disabled checkboxes?

42895-menu-assignment

@C-Lodder
Copy link
Member Author

C-Lodder commented Feb 26, 2024

@Quy - It wouldn't make the UX better to be honest. If a checkbox appears to be "disabled" (as shown in your screenshot), I'd assume I wouldn't be able to manually check it, which the user should be able to.

@Quy
Copy link
Contributor

Quy commented Feb 26, 2024

It is unchecked and not toggleble by default for URL, Alias, Heading, however, the All/None and Select/Deselect toggle them. Ideally, they should be skipped with these actions.

@C-Lodder
Copy link
Member Author

Oh I see. Didn't know that was the expected behavior. Will take a look tomorrow morning

@C-Lodder
Copy link
Member Author

@Quy fixed

@ghost
Copy link

ghost commented Feb 27, 2024

I have tested this item ✅ successfully on 1ce3ae6

Test by using prebuilt package:

image

PR having label NPM Resource Changed can't be tested by Patchtester.


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

@Quy
Copy link
Contributor

Quy commented Feb 27, 2024

I have tested this item ✅ successfully on 1ce3ae6

Thank you!!!


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

@Quy
Copy link
Contributor

Quy commented Feb 27, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 27, 2024
@laoneo
Copy link
Member

laoneo commented Mar 4, 2024

@C-Lodder can you fix the whitespaces?

@laoneo laoneo added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Mar 4, 2024
@C-Lodder
Copy link
Member Author

C-Lodder commented Mar 4, 2024

@laoneo Done.
Silly windows eol

@Quy Quy removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Mar 4, 2024
@laoneo
Copy link
Member

laoneo commented Mar 4, 2024

@C-Lodder thanks

@laoneo laoneo merged commit 3ad2ab9 into joomla:4.4-dev Mar 4, 2024
@laoneo
Copy link
Member

laoneo commented Mar 4, 2024

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 4, 2024
@laoneo laoneo added this to the Joomla! 4.4.4 milestone Mar 4, 2024
@C-Lodder C-Lodder deleted the treeselect branch September 24, 2024 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.4-dev

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants