[#33679] - [4.0] Fix the console errors
- Fixed in Code Base
- 9 May 2021
- Medium
- Build: 4.0-dev
- # 33679
- Diff
- dgrammatiko:—module-add—4.0-dev
User tests: Successful: Unsuccessful:
Pull Request for Issue #33652 .
Summary of Changes
Bug fixing
Testing Instructions
Click "Add module to the dashboard" on the Home Dashboard page. Look at JS Console.
Actual result BEFORE applying this Pull Request
Console errors
Expected result AFTER applying this Pull Request
No errors
Documentation Changes Required
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | 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
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33679.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33679.
I have tested this item
Really works now. Maybe I should have coffee, too, before testing :D
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33679.
I have tested this item
Thanks.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33679.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33679.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33679.
| 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: ? |
||
88.0 here and looking good. Maybe this was out of sync (I just updated it)?
@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.