-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[4.0] Update check alerts #24893
[4.0] Update check alerts #24893
Conversation
In J3 the quickicon for the new version etc where hidden in the bottom left of the control panel so we had a popup alert as well. As the quickicons are now bigger and right at the top we don't need the popup as well. This PR removes the popup and associated strings and ensures that the icons get the red danger instead of the brown warning and also standardises the 3 strings so that the count is in the same place.
Joomla.renderMessages(messages); | ||
|
||
// Scroll to page top | ||
window.scrollTo(0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're removing the scroll to top here (which is fine). Then we can remove the 'requirement' on windows from the top/bottom of the file for the anonymous function as we aren't using it anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry you will have to explain in dummy terms as I really dont understand js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you just mean change references of (window, document, Joomla);
to (document, Joomla);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes top and bottom of the file
@wilsonge I updated jupdatecheck.es6.js as suggested I didnt see anything in extensionupdatecheck.es6.js to update ?? |
Looks good to me. Yeah I'm not sure exactly why that is - i guess something is theoretically wrong somewhere there :/ but as long as it works :/ |
Thanks |
Note: |
What I meant is that the ideal for these checks would be to combine the Messages in a unique Warning message with as many items concerned as necessary. |
That was a side effect bonus of the issue that was being solved here. Please create your own issue for the messages |
@infograf768 there's definitely a bigger problem to solve in speed of messages being removed. However in this specific case we had both alerts and the warning buttons at the top of the screen. Which meant we had two alerts for the same thing. |
In J3 the quickicon for the new version etc where hidden in the bottom left of the control panel so we had a popup alert as well. As the quickicons are now bigger and right at the top we don't need the popup as well. Also you will see that the popups over wrote each other so they were even more useless
This PR removes the popup and associated strings and ensures that the icons get the red danger instead of the brown warning and also standardises the 3 strings so that the count is in the same place.
Testing Instructions
Will require npm i
To test extension updates - install a language and then change the version number in the db
To test joomla updates - create a custom extension server
Before
After