User tests: Successful: Unsuccessful:
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_templates |
Title |
|
Labels |
Added:
J4 Issue
?
|
@alikon @infograf768 @Quy please test this PR
This PR does not take into account the Toolbar
"Default"
If checkall is used, it should not be available i.e should be greyed
@infograf768
But in other components also like, com_menu, it is not "greyed"
Isn't this an expected behavior?
The problem Here is that listCheck
is boolean, i.e. we can't apparently force selecting only one item.
And, yes, I think we have a similar problem for com_menus when the menu items selected are assigned the same language (or ALL).
@infograf768 what could be a possible solution then?
I see that if one choses multiple template styles and use the Default button, then it is the one with the smaller id which is set as default. No message.
Therefore, I would just forget adding a checkall for template styles. It is too imprevisible.
We also don't do it for Languages.
Ok, I get your point, then How do I grey out Toolbar "Default", if check all is used?
As I said, I don’t think it is necessary. We don’t really need to use a checkall in this manager
But for the design of this component to be consistent with others, shouldn't this checkall be included?
The component should have the checkall.I have seen sites with several pages of template styles. And of course for a consistent ui.
@infograf768 is also correct that the "default" button should be disabled when multiple items are checked.
@brianteeman @infograf768 @bembelimen is there any component in which a toolbar button is disabled when multiple items are selected(to be used as a reference)
the toolbar has changed a lot in j4 so I'm not sure - you would have to check. I think in j3 the "home" button in the menu manager has the "check" functionality you are looking for although it didnt disable the button and showed a message instead
is there any component in which a toolbar button is disabled when multiple items are selected(to be used as a reference)
no. see above a screen shot of what we get for menu items.
I have also explained above the boolean aspect of “listCheck” which allows greying the button when nothing is selected but does not consider the number of ticked items.
One could create a specific query in the model throwing an alert when attempting to create multiple default styles.
We can at least get an alert message by using
$toolbar->confirmButton('default')
->text('COM_TEMPLATES_TOOLBAR_SET_HOME')
->message('COM_TEMPLATES_CONFIRM_SET_DEFAULT')
->task('styles.setDefault')
->listCheck(true);
The message would contain a text telling the user to make sure a unique style is chosen to be set as default.
We can at least get an alert message by using
@infograf768 which class should I include, to use confirmButton
?
use Joomla\CMS\Toolbar\Toolbar;
an then in the method
// Get the toolbar object instance
$toolbar = Toolbar::getInstance('toolbar');
btw, this could be done in another PR as using the Toolbar class only for that Toolbar button would be a waist. Also, I have not figured yet how to add an icon there.
Labels |
Removed:
J4 Issue
|
Category | Administration com_templates | ⇒ | Administration com_templates Language & Strings |
Please let's separate the stuff with the view...
@infograf768 I was able to add the icon
Also, I have included the changes suggested by you
As I said, the view needs more changes once we use the Toolbar class.
Let's make this in a separate PR
As I said, the view needs more changes once we use the Toolbar class.
Let's make this in a separate PR
Could you elaborate for what, the separate PR should be?
Could you elaborate for what the separate PR should be?
It would be ONLY for the View.
It would modify most of ToolbarHelper::
to use $toolbar->
I am on it now. Trying to find the way to also do it for duplicate.
In any case we do need a new string also for the Default message.
Could you elaborate for what the separate PR should be?
It would be ONLY for the View.
It would modify most ofToolbarHelper::
to use$toolbar->
I am on it now. Trying to find the way to also do it for duplicate.
In any case we do need a new string also for the Default message.
Should I revert the latest commit?
Should I revert the latest commit?
Yep. Let's keep this PR for its original purpose.
Labels |
Added:
?
|
Category | Administration com_templates Language & Strings | ⇒ | Administration com_templates |
Labels |
Removed:
?
|
@infograf768 kindly mark your test for this PR
I have tested this item
Will make the pr tomorrow for the view.
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-04-22 22:24:42 |
Closed_By | ⇒ | wilsonge |
Thanks!
done @Quy