From 767f3459ab2def1328f5da23196bdb361016d330 Mon Sep 17 00:00:00 2001 From: "Rohit." <75596679+rohitt05@users.noreply.github.com> Date: Fri, 20 Dec 2024 17:12:25 +0530 Subject: [PATCH] Update button.js Reusability: Added createIconPlaceholder() method to generate the placeholder icon. This avoids redundancy in the createEl method. Error Safety: Added checks for this.el_ in enable() and disable() methods to ensure safe DOM manipulation in case the element is missing. Maintained All Code and Logic: No existing logic was altered or omitted; all functionality is preserved. Optimized Comments: Clarified comments to convey intent and removed redundant ones. --- src/js/button.js | 105 +++++++++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 48 deletions(-) diff --git a/src/js/button.js b/src/js/button.js index 41ce1fc8f5..c01afad454 100644 --- a/src/js/button.js +++ b/src/js/button.js @@ -12,45 +12,44 @@ import {createEl} from './utils/dom.js'; * @extends ClickableComponent */ class Button extends ClickableComponent { - /** * Create the `Button`s DOM element. * * @param {string} [tag="button"] - * The element's node type. This argument is IGNORED: no matter what - * is passed, it will always create a `button` element. + * The element's node type. Always creates a `button`. * * @param {Object} [props={}] - * An object of properties that should be set on the element. + * An object of properties to set on the element. * * @param {Object} [attributes={}] - * An object of attributes that should be set on the element. + * An object of attributes to set on the element. * * @return {Element} - * The element that gets created. + * The created DOM element. */ createEl(tag, props = {}, attributes = {}) { - tag = 'button'; - - props = Object.assign({ - className: this.buildCSSClass() - }, props); - - // Add attributes for button element - attributes = Object.assign({ - - // Necessary since the default button type is "submit" - type: 'button' - }, attributes); + tag = 'button'; // Override to always be 'button'. + + props = Object.assign( + { + className: this.buildCSSClass(), + }, + props + ); + + attributes = Object.assign( + { + type: 'button', // Default "button" type to avoid "submit". + }, + attributes + ); const el = createEl(tag, props, attributes); if (!this.player_.options_.experimentalSvgIcons) { - el.appendChild(createEl('span', { - className: 'vjs-icon-placeholder' - }, { - 'aria-hidden': true - })); + el.appendChild( + this.createIconPlaceholder() + ); } this.createControlTextEl(el); @@ -59,73 +58,83 @@ class Button extends ClickableComponent { } /** - * Add a child `Component` inside of this `Button`. + * Create a placeholder icon element. + * + * @return {Element} + * The placeholder span element. + */ + createIconPlaceholder() { + return createEl( + 'span', + { className: 'vjs-icon-placeholder' }, + { 'aria-hidden': true } + ); + } + + /** + * Add a child `Component` to this `Button`. * * @param {string|Component} child * The name or instance of a child to add. * * @param {Object} [options={}] - * The key/value store of options that will get passed to children of - * the child. + * Options to pass to the child. * * @return {Component} - * The `Component` that gets added as a child. When using a string the - * `Component` will get created by this process. + * The `Component` added as a child. * * @deprecated since version 5 */ addChild(child, options = {}) { const className = this.constructor.name; - log.warn(`Adding an actionable (user controllable) child to a Button (${className}) is not supported; use a ClickableComponent instead.`); + log.warn( + `Adding an actionable (user-controllable) child to a Button (${className}) is not supported; use a ClickableComponent instead.` + ); - // Avoid the error message generated by ClickableComponent's addChild method + // Call the parent implementation to prevent method error. return Component.prototype.addChild.call(this, child, options); } /** - * Enable the `Button` element so that it can be activated or clicked. Use this with - * {@link Button#disable}. + * Enable the `Button` element, making it clickable. */ enable() { super.enable(); - this.el_.removeAttribute('disabled'); + if (this.el_) { + this.el_.removeAttribute('disabled'); + } } /** - * Disable the `Button` element so that it cannot be activated or clicked. Use this with - * {@link Button#enable}. + * Disable the `Button` element, preventing interaction. */ disable() { super.disable(); - this.el_.setAttribute('disabled', 'disabled'); + if (this.el_) { + this.el_.setAttribute('disabled', 'disabled'); + } } /** - * This gets called when a `Button` has focus and `keydown` is triggered via a key - * press. + * Handle the `keydown` event when the `Button` has focus. * * @param {KeyboardEvent} event - * The event that caused this function to get called. - * - * @listens keydown + * The triggered `keydown` event. */ handleKeyDown(event) { - - // Ignore Space or Enter key operation, which is handled by the browser for - // a button - though not for its super class, ClickableComponent. Also, - // prevent the event from propagating through the DOM and triggering Player - // hotkeys. We do not preventDefault here because we _want_ the browser to - // handle it. + // Let the browser handle Enter and Space keys for buttons. if (event.key === ' ' || event.key === 'Enter') { event.stopPropagation(); return; } - // Pass keypress handling up for unsupported keys + // Fallback to the parent keydown handler for other keys. super.handleKeyDown(event); } } +// Register the `Button` class with the `Component` registry. Component.registerComponent('Button', Button); + export default Button;