@Quy any idea whats creating the space before btn btn-primary?

Strange that there is a space before btn btn-primary, however removing this class could have side effects if the core or certain third-party extensions is using it to target a certain button... not sure if it's a good idea

See https://github.com/N6REJ/joomla-cms/blob/9256b24b9f702318831276790596eb6af96bfdbe/libraries/src/Toolbar/Button/StandardButton.php#L94

Strange that there is a space before btn btn-primary, however removing this class could have side effects if the core or certain third-party extensions is using it to target a certain button... not sure if it's a good idea

I hear your concern @joeforjoomla .
do we want to encourage bad class usage? The only other alternative is to rewrite the "standard" toolbar system completely.
I'm certainely open to suggestions.

@N6REJ having a button- class in the button element doesn't seem a bad class usage to me. If you think so... i don't care

@N6REJ having a button- class in the button element doesn't seem a bad class usage to me. If you think so... i don't care

I really don't think the attitude is necessary nor warranted. The class can be added IF NEEDED/DESIRED by the application simply by supplying an "button-icon-class" afaik.
to have button- there with the current J! core will require a rewrite of the toolbar class(s).

we could add a variable $button_class and make it the class for the <button> itself.
if we stripped the icon- and fa- from $icon then we could use
$button_class . ' ' . $icon in the toolbar class and that would satisfy all uses I would think.
Whats your thoughts?

@infograf768 This pr "fixes" the issue but in the process of fixing it I discovered another issue.
if we create the call w/o "fas fa-crop" and instead use NAME only.. such as
ToolbarHelper::custom('template.cropImage', 'crop', 'arrows-alt', 'COM_TEMPLATES_BUTTON_CROP', false);
then the problem goes away and all is well ( after this pr )
BUT if instead we use
ToolbarHelper::custom('template.cropImage', 'fas fa-crop', 'fas fa-arrows-alt', 'COM_TEMPLATES_BUTTON_CROP', false);
then the toolbar id has a space in it.

also icon-over breaks the whole thing if added back into custom() like it should be.. as currently written its never used.

imo its ready for testing now. There are design issues with icons but that has nothing to do with this issue/pr.

@joeforjoomla you'll be happy to know the button class is back

I have tested this item ✅ successfully on 6f09d50

I have tested this item ✅ successfully on 6f09d50

RTC

Tks