Skip to content
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] Replaced remaining js alerts for joomla alerts for the toolbar buttons #15330

Merged
merged 3 commits into from Apr 16, 2017
Merged

[4.0] Replaced remaining js alerts for joomla alerts for the toolbar buttons #15330

merged 3 commits into from Apr 16, 2017

Conversation

rjcf18
Copy link
Contributor

@rjcf18 rjcf18 commented Apr 16, 2017

Summary of Changes

As a followup to the Pull Request #15315 , besides the standard buttons I've noticed there were still buttons in the toolbar with the standard JS alert() on click when no item is selected, the batch button and the delete button, for instance, in Users. I've replaced the JS alert with the Joomla alert for those remaining buttons, I believe all buttons in the toolbar now have the Joomla alert on click when no item is selected.

Testing Instructions

Go to Banners and press Batch with no items selected and go to Users and press Delete with no items selected. In both cases you should see the Joomla alert instead of the JS alert().

Expected result

Toolbar buttons batch and toolbar standard buttons with a confirm dialog and Joomla alert on click.

deepinscreenshot20170416134643

deepinscreenshot20170416134712

Actual result

Toolbar buttons batch and toolbar standard buttons with a confirm dialog and standard JS alert() on click.

deepinscreenshot20170416132255

deepinscreenshot20170416132354

Documentation Changes Required

None

@brianteeman
Copy link
Contributor

Is it really correct to say "warning"

Warning Message

A warning message is information displayed warning the user about risks associated with the use of the item and may include a requirement to confirm before proceeding

Error Message

An error message is information displayed when an unexpected condition occurs,

@rjcf18
Copy link
Contributor Author

rjcf18 commented Apr 16, 2017

Oh right you're correct. Then how should I proceed here?

@rjcf18
Copy link
Contributor Author

rjcf18 commented Apr 16, 2017

Should I replace the warning messages with error messages?

@wilsonge
Copy link
Contributor

I think error makes more sense - shouldn't be hard to modify the existing PR to that :)

@rjcf18
Copy link
Contributor Author

rjcf18 commented Apr 16, 2017

Sure, I'll make that change ;)

@wilsonge
Copy link
Contributor

Thankyou very much :)

@wilsonge wilsonge merged commit 2158be7 into joomla:4.0-dev Apr 16, 2017
@wilsonge wilsonge added this to the Joomla 4.0 milestone Apr 16, 2017
@wilsonge
Copy link
Contributor

Good work :) Merged

@rjcf18
Copy link
Contributor Author

rjcf18 commented Apr 16, 2017

Great! Thx! ;)

@rjcf18
Copy link
Contributor Author

rjcf18 commented Apr 16, 2017

Oh missed the WARNING there in the message title in the standard buttons part in the JText::script(); here:

JText::script('WARNING');

@rjcf18
Copy link
Contributor Author

rjcf18 commented Apr 16, 2017

Should I do another PR to fix this? Sorry, totally missed that one O.o

@wilsonge
Copy link
Contributor

Don't worry :) Fixed it directly with 61c7673

@rjcf18
Copy link
Contributor Author

rjcf18 commented Apr 16, 2017

Ah ok thx ;) And thx to @NunoLopes96 for pointing out this improvement in the first place in his original PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants