User tests: Successful: Unsuccessful:
Pull Request for Issue #33652 .
Bug fixing
Click "Add module to the dashboard" on the Home Dashboard page. Look at JS Console.
Console errors
No errors
Status | New | ⇒ | Pending |
Category | ⇒ | JavaScript Repository NPM Change |
Labels |
Added:
NPM Resource Changed
?
|
Category | JavaScript Repository NPM Change | ⇒ | Administration com_modules JavaScript Repository NPM Change |
@joomdonation so I added back the alert part. BTW this PR also improves/simplifies the JS
Thanks @dgrammatiko . Will test it soon.
I have tested this item
I have tested this item
I have tested this item
Really works now. Maybe I should have coffee, too, before testing :D
I have tested this item
Thanks.
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Labels |
Added:
?
|
@dgrammatiko With current 4.0-dev using Firefox on Windows I don't get any errors in browser console also without this PR applied. Can it be the errors have been already fixed with another, recently merged PR, maybe #32657 ? Or did they only occur on certain browsers?
@richard67 what's your ff version?
88.0.1 (64-Bit) on Windows 10.
Labels |
Added:
?
Removed: ? |
@dgrammatiko I'd have no problem with merging this PR if it still makes sense, e.g. because improving code quality. I just like to understand that. It just seems there are no JS errors anymore to be fixed by it.
@richard67 oops my bad, misread what you wrote. So the problem was that there was a call to an element that didn't exist in the dom and calling foo.classList
when foo is undefined will always throw an error. The code is bringing back the missing dom element but also introduces conditionals so it will not break if some dom element goes missing...
@dgrammatiko Seems I was too much driven by the PR title "Fix the console errors" and so did not get why in addition the PR adds an alert. But I get it now, I've just tested. Without the PR, the "There are no modules matching your query." is missing, so it had to be added back here.
Anyway code review looks good to me so far with my little js knowledge. Stuff is checked if defined before using it.
Thanks!
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-05-09 14:44:55 |
Closed_By | ⇒ | richard67 | |
Labels |
Added:
?
Removed: ? |
Once we added the losing HTML code from this PR #32657 back, this PR would become invalid.