? ? NPM Resource Changed Pending

User tests: Successful: Unsuccessful:

avatar rjharishabh
rjharishabh
5 Oct 2021

Pull Request for Issue #35377.

Summary of Changes

Add if to check and respond only when the button is not disabled

Testing Instructions

please see issue

Don't forget to build JS npm run build:js

Actual result BEFORE applying this Pull Request

Disabled buttons respond

Expected result AFTER applying this Pull Request

Disabled buttons do not respond

preview.mp4

Documentation Changes Required

No

avatar rjharishabh rjharishabh - open - 5 Oct 2021
avatar rjharishabh rjharishabh - change - 5 Oct 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 5 Oct 2021
Category JavaScript Repository NPM Change
avatar rjharishabh rjharishabh - change - 5 Oct 2021
Labels Added: ? NPM Resource Changed
avatar brianteeman
brianteeman - comment - 5 Oct 2021

I have tested this item ? unsuccessfully on b632dce

obviously incorrect


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

avatar brianteeman brianteeman - test_item - 5 Oct 2021 - Tested unsuccessfully
avatar rjharishabh
rjharishabh - comment - 5 Oct 2021

Hi Brian, is it not working or any other issue

avatar brianteeman
brianteeman - comment - 5 Oct 2021

Try it and you will see

avatar brianteeman
brianteeman - comment - 5 Oct 2021

still broken

avatar brianteeman
brianteeman - comment - 6 Oct 2021

I see that I have to be more explicit - I was hoping that you would see at least one of the errors yourself.

  1. In this PR you are reverting a recent change in the language string. As a result if you actually try this code on the current repository then the messages are not translated.

Example

image

  1. As you have correctly disabled the ability to click on a disabled button then the condition that would cause the message to select one version can never be met. If that was intended then the code can be greatly simplified.
avatar rjharishabh
rjharishabh - comment - 7 Oct 2021
  1. In this PR you are reverting a recent change in the language string. As a result, if you actually try this code on the current repository then the messages are not translated.

Updated Thank you
Actually, I have not pulled latest changes from Github ?

avatar rjharishabh
rjharishabh - comment - 7 Oct 2021
  1. As you have correctly disabled the ability to click on a disabled button then the condition that would cause the message to select one version can never be met. If that was intended then the code can be greatly simplified.

When we select more than one version to restore or preview, then this message will help Please select one version.
Similarly, when we select one or three instead of two to compare, alert will be displayed Please select two versions.

avatar jwaisner
jwaisner - comment - 18 Oct 2021

@rjharishabh , I tested and now the disabled buttons no longer do anything but the error you got previously regarding "Please select one version" does not appear. Also the disabled icon does not seem to show by the cursor at all.


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

avatar rjharishabh
rjharishabh - comment - 18 Oct 2021

but the error you got previously regarding "Please select one version" does not appear.

Select two versions to preview, then this message will be displayed.

avatar rjharishabh
rjharishabh - comment - 18 Oct 2021

I've updated the description with a video.

avatar rjharishabh rjharishabh - change - 18 Oct 2021
The description was changed
avatar rjharishabh rjharishabh - edited - 18 Oct 2021
avatar richard67
richard67 - comment - 23 Oct 2021

@rjharishabh The description in issue #35377 mentioned 2 things in expected result:

  • If a button is disabled - clicking ANYWHERE on that button should be disabled.
  • If a button is disabled - hovering ANYWHERE on that button should show a disabled cursor
    But your PR seems to implement only the first one. At least I can't see any pointer change when hovering the buttons. Is that by purpose? Or is that something browser-specific not to be solved with a PR?
avatar rjharishabh
rjharishabh - comment - 23 Oct 2021

I have not done anything here for the disabled cursor
In the issue, the disabled icon is shown in Phil's video and with safari on mac
It may be a browser-specific thing, but I am not sure

avatar richard67
richard67 - comment - 23 Oct 2021

I have not done anything here for the disabled cursor In the issue, the disabled icon is shown in Phil's video and with safari on mac It may be a browser-specific thing, but I am not sure

@rjharishabh Firefox, Chrome and Edge do not do that, so maybe it is a Safari thing or @PhilETaylor wanted to have some css for that purpose. But for me this PR here is sufficient as it is, because the buttons are properly disabled in HTML when I inspect them, and the pointer does not change like it would when there was an enabled button or link, so it is clear that these buttons are disabled.

@PhilETaylor Please report back if you did expect more to be done for solving your issue. Thanks in advance.

avatar richard67
richard67 - comment - 23 Oct 2021

I have tested this item successfully on 342831d

The buttons are properly disabled now when nothing is selected.

If a wrong number of versions is selected, i.e. one version or more than 2 versions for the Compare button or 2 or more versions for the Preview button, the right error message is still show like it was before.


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

avatar richard67 richard67 - test_item - 23 Oct 2021 - Tested successfully
avatar RickR2H
RickR2H - comment - 24 Oct 2021

I have tested this item successfully on 342831d

Bug confirmed and patch is working!


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

avatar RickR2H RickR2H - test_item - 24 Oct 2021 - Tested successfully
avatar richard67 richard67 - change - 24 Oct 2021
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 24 Oct 2021

RTC


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

avatar wilsonge wilsonge - change - 28 Nov 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-11-28 17:30:58
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 28 Nov 2021
avatar wilsonge wilsonge - merge - 28 Nov 2021
avatar wilsonge
wilsonge - comment - 28 Nov 2021

Thanks!

Add a Comment

Login with GitHub to post a comment