[4.0][RFC] Custom Element: Joomla! Toolbar button#19635
[4.0][RFC] Custom Element: Joomla! Toolbar button#19635Fedik wants to merge 28 commits intojoomla:4.0-devfrom
Conversation
|
@Fedik one thing that would be awesome to implement is hide/show buttons depending on the selected rows of the list view. I started working on that idea so maybe something helpful there: https://github.com/joomla/joomla-cms/compare/4.0-dev...dgt41:§4.0-dev-toolbar-cleanup?expand=1 PS And yes this is needed to achieve CSP strict mode! Great stuff |
|
|
||
| if (form.boxchecked.value == 0) { | ||
| perform = false; | ||
| Joomla.renderMessages({'error': [Joomla.JText._('JLIB_HTML_PLEASE_MAKE_A_SELECTION_FROM_THE_LIST')]}); |
There was a problem hiding this comment.
how about: this.setAttribute('hidden', true); instead of the stupid alert?
There was a problem hiding this comment.
Of course you need a way to exclude buttons from the hide/show behaviour (eg new, options, etc)
There was a problem hiding this comment.
There was a problem hiding this comment.
for now it just keeps an old logic
There was a problem hiding this comment.
I'm fine let's do this in steps...
|
One issue with the current code we are using is that the "icon" name is tied to the button id. So for example in the sysinfo there is an a11y fail as the id is the same on both buttons because the icon is the same. it would be good if that could be avoided with this new code |
that also would be cool, |
|
@Fedik one more thing about accessibility: |
|
@DGT41 good point, need to check |
|
well it's not only tabindex you need to patch the key listeners as well. Basically because custom element |
|
yes, that was another idea in my head, |
|
|
||
| // If list selection are required, set button to disabled by default | ||
| if (this.listConfirmation) { | ||
| button.setAttribute('disabled', 'disabled'); |
There was a problem hiding this comment.
button.setAttribute('disabled', true);
| Joomla.submitbutton(this.task, form, this.formValidation); | ||
| // Check whether we have selected something | ||
| if (this.formElement.boxchecked.value == 0) { | ||
| this.buttonElement.setAttribute('disabled', 'disabled'); |
There was a problem hiding this comment.
button.setAttribute('disabled', true);
|
@brianteeman the issue with non-unique ID is fixed @DGT41 any idea why the link button use |
| * @param boolean $list True to allow lists | ||
| * @param boolean $group Does the button belong to a group? | ||
| * | ||
| * @param string $form The form CSS selector eg '#adminForm' |
| * @param string $alt An override for the alt text. | ||
| * @param boolean $group True or false | ||
| * | ||
| * @param string $form The form ID |
|
@Fedik I guess the |
|
@DGT41 hmm, no should be something diferent, because the link not require a list selection: I could change it to simple |
|
Just reference I create a PR #19670 to handle the php side. |
|
@Fedik one more thing here, a few months back I implemented a shortcut key handler for CTRL( or CMD)+S. That code somehow disappeared in some sync of the repos but you can check it here: joomla/40-backend-template#55. So it would be cool to have it back, probably it needs some changes as the codebase progressed a bit since |
|
@DGT41 I swear you've added that feature like 4 times now |
|
@DGT41 I have restored "control+s" However if we can use one of these: |
media/system/js/core.js
Outdated
| document.addEventListener( 'DOMContentLoaded', function() { | ||
| if (Joomla.getOptions( 'keySave' ) ) { | ||
| document.addEventListener("keydown", function(e) { | ||
| if (e.keyCode == 83 && (navigator.platform.match("Mac") ? e.metaKey : e.ctrlKey)) { |
There was a problem hiding this comment.
Please use https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key in favour of keyCode
|
@Fedik the code was in core.js because we didn't have a toolbar.js or had a clue that custom elements was going to be part of J4 back then. Now the approach should be a little different IMHO: const keyboardSpecialKeys = (key, modifier, task) => {
if (modifier === 'control') { modifier = (navigator.platform.match("Mac") ? e.metaKey : e.ctrlKey); }
document.addEventListener("keydown", function(e) {
if (e.key === key && e.key === modifier ) {
e.preventDefault();
Joomla.submitbutton(task); // Pretty sure we can use submitForm here
}
}, false);
}in the class connectedCallback function: keyboardSpecialKeys(83, 'control', 'article.save')Or you can use |
|
I would not put too much stuff into this. But we should improve this a little:
|
Conflicts: media/system/js/core.min.js
I think it is fine,
There could be 3 db entries 😉
That actually what we have here
I would not do a Zoo of "toolbar-buttons" which on 90% will be copy of each other.
It is not very coupled. |
layouts/joomla/toolbar/apply.php
Outdated
| $group = $displayData['group']; | ||
| $task = ''; | ||
| $list = !empty($displayData['list']) ? ' list-selection' : ''; | ||
| $form = !empty($displayData['form']) ? ' form=' . $displayData['form'] . '"' : ''; |
|
I've merged the PHP API Updates which of course has made this blow up with conflicts. Can we get this back in sync |
|
I will check |
|
I made new one, please check there #20650 |

This is sketch of
<joomla-toolbar-button/>.It allow us to move most of inline
onclick=""to separate file.From JavaScript side it mostly ready, but PHP require some more editing.
Any thoughts, is it need?
ping @DGT41 @C-Lodder