[4.0][RFC] Custom Element: Joomla! Toolbar button#20650
[4.0][RFC] Custom Element: Joomla! Toolbar button#20650wilsonge merged 12 commits intojoomla:4.0-devfrom
Conversation
|
|
||
| customElements.define('joomla-toolbar-button', JoomlaToolbarButton); | ||
|
|
||
| })(customElements); |
There was a problem hiding this comment.
Block must not be padded by blank lines padded-blocks
|
|
||
| } | ||
|
|
||
| customElements.define('joomla-toolbar-button', JoomlaToolbarButton); |
There was a problem hiding this comment.
Expected indentation of 2 spaces but found 4 indent
|
|
||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Expected indentation of 2 spaces but found 4 indent
Block must not be padded by blank lines padded-blocks
| throw new Error('Either "task" or "execute" attribute must be preset to perform an action.'); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Expected indentation of 4 spaces but found 8 indent
Block must not be padded by blank lines padded-blocks
| method.call({}); | ||
| } else { | ||
| throw new Error('Either "task" or "execute" attribute must be preset to perform an action.'); | ||
| } |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 12 indent
| return; | ||
| } | ||
|
|
||
| if (this.task) { |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 12 indent
|
|
||
| if (this.confirmMessage && !confirm(this.confirmMessage)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 12 indent
| } | ||
|
|
||
| if (this.confirmMessage && !confirm(this.confirmMessage)) { | ||
| return; |
There was a problem hiding this comment.
Expected indentation of 8 spaces but found 16 indent
| return; | ||
| } | ||
|
|
||
| if (this.confirmMessage && !confirm(this.confirmMessage)) { |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 12 indent
Unexpected use of 'confirm' no-restricted-globals
| executeTask() { | ||
| if (this.disabled) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 12 indent
Update toolbar-button.js
| if (this.task) { | ||
| Joomla.submitbutton(this.task, this.form, this.formValidation); | ||
| } else if (this.execute) { | ||
| let method = new Function(this.execute); |
There was a problem hiding this comment.
'method' is never reassigned. Use 'const' instead prefer-const
The Function constructor is eval no-new-func
| return; | ||
| } | ||
|
|
||
| if (this.confirmMessage && !confirm(this.confirmMessage)) { |
There was a problem hiding this comment.
Unexpected use of 'confirm' no-restricted-globals
|
|
||
| if (this.listSelection) { | ||
| if (!this.formElement) { | ||
| throw new Error('The form "' + formSelector + '" is required to perform the task, but the form not found on the page.'); |
There was a problem hiding this comment.
Unexpected string concatenation prefer-template
| this.addEventListener('click', this.executeTask); | ||
|
|
||
| // Check whether we have a form | ||
| let formSelector = this.form || '#adminForm'; |
There was a problem hiding this comment.
'formSelector' is never reassigned. Use 'const' instead prefer-const
|
|
||
| // Bind neccessary functions | ||
| this.executeTask = this.executeTask.bind(this); | ||
| setDisabled = setDisabled.bind(this); |
There was a problem hiding this comment.
'setDisabled' is not defined no-undef
| constructor() { | ||
| super(); | ||
|
|
||
| // We need a button to support button behavior, because we cannot currently extend HTMLButtonElement |
There was a problem hiding this comment.
Line 20 exceeds the maximum line length of 100 max-len
| * @license GNU General Public License version 2 or later; see LICENSE.txt | ||
| */ | ||
| (function (customElements) { | ||
| "use strict"; |
There was a problem hiding this comment.
Strings must use singlequote quotes
| * @copyright Copyright (C) 2005 - 2018 Open Source Matters, Inc. All rights reserved. | ||
| * @license GNU General Public License version 2 or later; see LICENSE.txt | ||
| */ | ||
| (function (customElements) { |
There was a problem hiding this comment.
Move the invocation into the parens that contain the function wrap-iife
Unexpected unnamed function func-names
|
@dgrammatiko thanks |
|
Did not remark any change in toolbar behavior after patch. |
| if (this.task) { | ||
| Joomla.submitbutton(this.task, this.form, this.formValidation); | ||
| } else if (this.execute) { | ||
| const method = new Function(this.execute); |
There was a problem hiding this comment.
The Function constructor is eval no-new-func
| return false; | ||
| } | ||
|
|
||
| if (this.confirmMessage && !confirm(this.confirmMessage)) { |
There was a problem hiding this comment.
Unexpected use of 'confirm' no-restricted-globals
| } | ||
| } | ||
|
|
||
| executeTask() { |
There was a problem hiding this comment.
Expected to return a value at the end of method 'executeTask' consistent-return
| // Watch on list selection | ||
| this.formElement.boxchecked.addEventListener('change', (event) => { | ||
| // Check whether we have selected something | ||
| this.setDisabled(event.target.value == 0); |
There was a problem hiding this comment.
Expected '===' and instead saw '==' eqeqeq
| this.setDisabled(true); | ||
| } | ||
|
|
||
| this.addEventListener('click', e => this.executeTask()); |
There was a problem hiding this comment.
'e' is defined but never used no-unused-vars
| // Attribute getters | ||
| get task() {return this.getAttribute('task');} | ||
| get execute() {return this.getAttribute('execute');} | ||
| get listSelection() {return this.hasAttribute('list-selection');} |
There was a problem hiding this comment.
Requires a space after '{' block-spacing
Multiple spaces found before '{' no-multi-spaces
Requires a space before '}' block-spacing
|
|
||
| // Attribute getters | ||
| get task() {return this.getAttribute('task');} | ||
| get execute() {return this.getAttribute('execute');} |
There was a problem hiding this comment.
Multiple spaces found before '{' no-multi-spaces
Requires a space after '{' block-spacing
Requires a space before '}' block-spacing
| class JoomlaToolbarButton extends HTMLElement { | ||
|
|
||
| // Attribute getters | ||
| get task() {return this.getAttribute('task');} |
There was a problem hiding this comment.
Multiple spaces found before '{' no-multi-spaces
Requires a space after '{' block-spacing
Requires a space before '}' block-spacing
| ;((customElements) => { | ||
| 'use strict'; | ||
|
|
||
| class JoomlaToolbarButton extends HTMLElement { |
There was a problem hiding this comment.
Block must not be padded by blank lines padded-blocks
| * @license GNU General Public License version 2 or later; see LICENSE.txt | ||
| */ | ||
|
|
||
| ;((customElements) => { |
There was a problem hiding this comment.
Unnecessary semicolon no-extra-semi
|
|
||
| customElements.define('joomla-toolbar-button', JoomlaToolbarButton); | ||
|
|
||
| })(customElements); |
There was a problem hiding this comment.
Block must not be padded by blank lines padded-blocks
| return true; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Block must not be padded by blank lines padded-blocks
| return false; | ||
| } | ||
|
|
||
| if (this.confirmMessage && !confirm(this.confirmMessage)) { |
There was a problem hiding this comment.
Unexpected use of 'confirm' no-restricted-globals
| // We need a button to support button behavior, | ||
| // because we cannot currently extend HTMLButtonElement | ||
| this.buttonElement = this.querySelector('button'); | ||
| this.disabled = false; |
There was a problem hiding this comment.
Multiple spaces found before '=' no-multi-spaces
| get listSelection() {return this.hasAttribute('list-selection');} | ||
| get form() {return this.getAttribute('form');} | ||
| get formValidation() {return this.hasAttribute('form-validation');} | ||
| get confirmMessage() {return this.getAttribute('confirm-message');} |
There was a problem hiding this comment.
Requires a space after '{' block-spacing
Requires a space before '}' block-spacing
| get task() {return this.getAttribute('task');} | ||
| get listSelection() {return this.hasAttribute('list-selection');} | ||
| get form() {return this.getAttribute('form');} | ||
| get formValidation() {return this.hasAttribute('form-validation');} |
There was a problem hiding this comment.
Requires a space after '{' block-spacing
Requires a space before '}' block-spacing
| // Attribute getters | ||
| get task() {return this.getAttribute('task');} | ||
| get listSelection() {return this.hasAttribute('list-selection');} | ||
| get form() {return this.getAttribute('form');} |
There was a problem hiding this comment.
Requires a space after '{' block-spacing
Requires a space before '}' block-spacing
|
|
||
| // Attribute getters | ||
| get task() {return this.getAttribute('task');} | ||
| get listSelection() {return this.hasAttribute('list-selection');} |
There was a problem hiding this comment.
Requires a space after '{' block-spacing
Requires a space before '}' block-spacing
| class JoomlaToolbarButton extends HTMLElement { | ||
|
|
||
| // Attribute getters | ||
| get task() {return this.getAttribute('task');} |
There was a problem hiding this comment.
Requires a space after '{' block-spacing
Requires a space before '}' block-spacing
| ((customElements) => { | ||
| 'use strict'; | ||
|
|
||
| class JoomlaToolbarButton extends HTMLElement { |
There was a problem hiding this comment.
Block must not be padded by blank lines padded-blocks
| return false; | ||
| } | ||
|
|
||
| if (this.confirmMessage && !confirm(this.confirmMessage)) { |
There was a problem hiding this comment.
Unexpected use of 'confirm' no-restricted-globals
There was a problem hiding this comment.
this was expected. deal with it 
There was a problem hiding this comment.
told hound to deal with it :)
Conflicts: media/system/js/core.min.js
|
Awesome work! Thanks for being so patient with this one! |
This is redo of #19635
This patch add
<joomla-toolbar-button/>custom element, to standard toolbar button.It allow us to move most of inline onclick="" to separate file.
Currently it only for
StandardButton.Testing Instructions
Click the toolbar buttons here and there, and make sure all works as expected, as before the patch.
ping @dgrammatiko @wilsonge