Skip to content

Commit

Permalink
fix(FEC-10144): player causes form submit on button click (#512)
Browse files Browse the repository at this point in the history
When the player is nested within a form when clicking any button that is in the control bar we get redirected to any form referral.
  • Loading branch information
Shalom Meoded committed Jun 7, 2020
1 parent 2d9df3b commit 3982da6
Show file tree
Hide file tree
Showing 20 changed files with 70 additions and 40 deletions.
15 changes: 15 additions & 0 deletions src/components/button/button.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@flow
import {h} from 'preact';
import {forwardRef} from 'preact/compat';

const COMPONENT_NAME = 'Button';
/**
* Button component
*
* @const Button
* @example <Button/>
*/
const Button = forwardRef((props, ref) => <button type="button" ref={ref} {...props} />);

Button.displayName = COMPONENT_NAME;
export {Button};
1 change: 1 addition & 0 deletions src/components/button/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export {Button} from './button';
5 changes: 3 additions & 2 deletions src/components/cast-on-tv/cast-before-play.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {Icon} from '../icon/icon';
import {Localizer, Text} from 'preact-i18n';
import {withPlayer} from '../player';
import {withLogger} from 'components/logger';
import {Button} from 'components/button';

/**
* mapping state to props
Expand Down Expand Up @@ -86,7 +87,7 @@ class CastBeforePlay extends Component {
<div>
<div className={rootStyle.join(' ')}>
<Localizer>
<button
<Button
tabIndex="0"
aria-label={<Text id={'cast.play_on_tv'} />}
onClick={() => this.onClick()}
Expand All @@ -97,7 +98,7 @@ class CastBeforePlay extends Component {
<span>
<Text id="cast.play_on_tv" />
</span>
</button>
</Button>
</Localizer>
</div>
</div>
Expand Down
5 changes: 3 additions & 2 deletions src/components/error-overlay/error-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {actions} from '../../reducers/engine';
import {CopyButton} from '../copy-button';
import {withLogger} from 'components/logger';
import {withPlayer} from 'components/player';
import {Button} from 'components/button';

/**
* mapping state to props
Expand Down Expand Up @@ -101,9 +102,9 @@ class ErrorOverlay extends Component {
if (this.props.player.getMediaInfo()) {
return (
<div className={style.controlButtonContainer} onClick={() => this.handleClick()}>
<button className={[style.controlButton, style.retryBtn].join(' ')}>
<Button className={[style.controlButton, style.retryBtn].join(' ')}>
<Text id="error.retry" />
</button>
</Button>
</div>
);
}
Expand Down
5 changes: 3 additions & 2 deletions src/components/forward/forward.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {withPlayer} from '../player';
import {withEventDispatcher} from 'components/event-dispatcher';
import {withLogger} from 'components/logger';
import {Tooltip} from 'components/tooltip';
import {Button} from 'components/button';

const COMPONENT_NAME = 'Forward';

Expand Down Expand Up @@ -66,14 +67,14 @@ class Forward extends Component {
return (
<div className={[style.controlButtonContainer, style.noIdleControl].join(' ')}>
<Tooltip label={this.props.forwardText}>
<button
<Button
tabIndex="0"
aria-label={this.props.forwardText}
className={`${style.controlButton}`}
ref={this.props.innerRef}
onClick={() => this.onClick()}>
<Icon type={!props.step || props.step === FORWARD_DEFAULT_STEP ? IconType.Forward10 : IconType.Forward} />
</button>
</Button>
</Tooltip>
</div>
);
Expand Down
5 changes: 3 additions & 2 deletions src/components/fullscreen/fullscreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {withKeyboardEvent} from 'components/keyboard';
import {withLogger} from 'components/logger';
import {Tooltip} from 'components/tooltip';
import {withEventDispatcher} from 'components/event-dispatcher';
import {Button} from 'components/button';

/**
* mapping state to props
Expand Down Expand Up @@ -115,14 +116,14 @@ class Fullscreen extends Component {
return (
<div className={[style.controlButtonContainer, style.controlFullscreen].join(' ')}>
<Tooltip label={this.props.fullscreenText}>
<button
<Button
tabIndex="0"
aria-label={this.props.fullscreenText}
className={this.props.fullscreen ? [style.controlButton, style.isFullscreen].join(' ') : style.controlButton}
onClick={() => this.toggleFullscreen()}>
<Icon type={IconType.Maximize} />
<Icon type={IconType.Minimize} />
</button>
</Button>
</Tooltip>
</div>
);
Expand Down
1 change: 1 addition & 0 deletions src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export {PlaylistButton} from './playlist-button';
export {PlaylistNextScreen} from './playlist-next-screen';
export {PictureInPicture} from './picture-in-picture';
export {PlaybackControls} from './playback-controls';
export {Button} from './button';

export {Keyboard as KeyboardControl} from './keyboard';
export {Cast, Cast as CastControl};
Expand Down
5 changes: 3 additions & 2 deletions src/components/language/language.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {withEventDispatcher} from 'components/event-dispatcher';
import {KeyMap} from 'utils/key-map';
import {withKeyboardEvent} from 'components/keyboard';
import {Tooltip} from 'components/tooltip';
import {Button} from 'components/button';

/**
* mapping state to props
Expand Down Expand Up @@ -207,14 +208,14 @@ class Language extends Component {
return (
<div ref={c => (this._controlLanguageElement = c)} className={[style.controlButtonContainer, style.controlLanguage].join(' ')}>
<Tooltip label={this.props.buttonLabel}>
<button
<Button
tabIndex="0"
aria-haspopup="true"
aria-label={this.props.buttonLabel}
className={this.state.smartContainerOpen ? [style.controlButton, style.active].join(' ') : style.controlButton}
onClick={() => this.onControlButtonClick()}>
<Icon type={IconType.Language} />
</button>
</Button>
</Tooltip>
{!this.state.smartContainerOpen || this.state.cvaaOverlay ? (
undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {connect} from 'react-redux';
import {Localizer, Text} from 'preact-i18n';
import {withPlayer} from '../player';
import {withLogger} from 'components/logger';

import {Button} from 'components/button';
/**
* mapping state to props
* @param {*} state - redux store state
Expand Down Expand Up @@ -74,9 +74,9 @@ class PictureInPictureOverlay extends Component {
</span>
</Localizer>
<Localizer>
<button tabIndex="0" className={[style.pictureInPictureButton, style.controlButton].join(' ')} onClick={() => this._handleClick()}>
<Button tabIndex="0" className={[style.pictureInPictureButton, style.controlButton].join(' ')} onClick={() => this._handleClick()}>
<Text id="pictureInPicture.overlay_button" />
</button>
</Button>
</Localizer>
</div>
</div>
Expand Down
5 changes: 3 additions & 2 deletions src/components/picture-in-picture/picture-in-picture.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {KeyMap} from 'utils/key-map';
import {withKeyboardEvent} from 'components/keyboard';
import {withEventDispatcher} from 'components/event-dispatcher';
import {Tooltip} from 'components/tooltip';
import {Button} from 'components/button';

/**
* mapping state to props
Expand Down Expand Up @@ -87,13 +88,13 @@ class PictureInPicture extends Component {
return (
<div className={[style.controlButtonContainer, style.pictureInPicture].join(' ')}>
<Tooltip label={this.props.pipText}>
<button
<Button
tabIndex="0"
aria-label={this.props.pipText}
className={`${style.controlButton} ${this.state.animation ? style.rotate : ''}`}
onClick={() => this.togglePip()}>
<Icon type={IconType.PictureInPicture} />
</button>
</Button>
</Tooltip>
</div>
);
Expand Down
5 changes: 3 additions & 2 deletions src/components/play-pause/play-pause.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {bindActions} from 'utils/bind-actions';
import {actions as settingActions} from 'reducers/settings';
import {actions as overlayIconActions} from 'reducers/overlay-action';
import {Tooltip} from 'components/tooltip';
import {Button} from 'components/button';

/**
* mapping state to props
Expand Down Expand Up @@ -98,7 +99,7 @@ class PlayPause extends Component {
return (
<div className={[style.controlButtonContainer, style.controlPlayPause].join(' ')}>
<Tooltip label={labelText}>
<button tabIndex="0" aria-label={labelText} className={controlButtonClass} onClick={() => this.togglePlayPause()}>
<Button tabIndex="0" aria-label={labelText} className={controlButtonClass} onClick={() => this.togglePlayPause()}>
{isStartOver ? (
<Icon type={IconType.StartOver} />
) : (
Expand All @@ -107,7 +108,7 @@ class PlayPause extends Component {
<Icon type={IconType.Pause} />
</div>
)}
</button>
</Button>
</Tooltip>
</div>
);
Expand Down
6 changes: 3 additions & 3 deletions src/components/playlist-button/playlist-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {default as Icon, IconType} from '../icon';
import {connect} from 'react-redux';
import {withPlayer} from '../player';
import {Tooltip} from 'components/tooltip';

import {Button} from 'components/button';
/**
* mapping state to props
* @param {*} state - redux store state
Expand Down Expand Up @@ -86,7 +86,7 @@ class PlaylistButton extends Component {
*/
bottomBarButton(item: any, type: string): React$Element<any> {
return (
<button
<Button
disabled={!item}
tabIndex="0"
aria-label={this.props[`${type}ControlsText`]}
Expand All @@ -101,7 +101,7 @@ class PlaylistButton extends Component {
<Icon type={IconType.Next} />
</div>
)}
</button>
</Button>
);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/components/playlist-countdown/playlist-countdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {withLogger} from 'components/logger';
import {bindActions} from '../../utils/bind-actions';
import {actions} from 'reducers/playlist';
import {withEventManager} from 'event/with-event-manager';

import {Button} from 'components/button';
/**
* mapping state to props
* @param {*} state - redux store state
Expand Down Expand Up @@ -263,7 +263,7 @@ class PlaylistCountdown extends Component {
</Localizer>
<div className={[style.controlButtonContainer, style.playlistCountdownCancel].join(' ')}>
<Localizer>
<button
<Button
tabIndex={this.state.focusable ? 0 : -1}
aria-label={<Text id="playlist.cancel" />}
className={[style.controlButton, style.playlistCountdownCancelButton].join(' ')}
Expand All @@ -274,7 +274,7 @@ class PlaylistCountdown extends Component {
}
}}>
<Icon type={IconType.Close} />
</button>
</Button>
</Localizer>
</div>
<div className={style.playlistCountdownIndicatorBar}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {withPlayer} from '../player';
import {withEventDispatcher} from 'components/event-dispatcher';
import {withLogger} from 'components/logger';
import {Tooltip} from 'components/tooltip';
import {Button} from 'components/button';

/**
* mapping state to props
Expand Down Expand Up @@ -71,7 +72,7 @@ class PrePlaybackPlayOverlay extends Component {
const labelText = props.isPlaybackEnded ? props.startOverText : props.playText;
return (
<div className={style.prePlaybackPlayOverlay} onMouseOver={e => e.stopPropagation()} onClick={() => this.handleClick()}>
<button
<Button
className={style.prePlaybackPlayButton}
tabIndex="0"
aria-label={labelText}
Expand All @@ -81,7 +82,7 @@ class PrePlaybackPlayOverlay extends Component {
}
}}>
<Tooltip label={labelText}>{props.isPlaybackEnded ? <Icon type={IconType.StartOver} /> : <Icon type={IconType.Play} />}</Tooltip>
</button>
</Button>
</div>
);
}
Expand Down
5 changes: 3 additions & 2 deletions src/components/rewind/rewind.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {withPlayer} from '../player';
import {withEventDispatcher} from 'components/event-dispatcher';
import {withLogger} from 'components/logger';
import {Tooltip} from 'components/tooltip';
import {Button} from 'components/button';

const COMPONENT_NAME = 'Rewind';

Expand Down Expand Up @@ -65,14 +66,14 @@ class Rewind extends Component {
return (
<div className={[style.controlButtonContainer, style.noIdleControl].join(' ')}>
<Tooltip label={this.props.rewindText}>
<button
<Button
tabIndex="0"
aria-label={this.props.rewindText}
className={`${style.controlButton}`}
ref={this.props.innerRef}
onClick={() => this.onClick()}>
<Icon type={!props.step || props.step === REWIND_DEFAULT_STEP ? IconType.Rewind10 : IconType.Rewind} />
</button>
</Button>
</Tooltip>
</div>
);
Expand Down
5 changes: 3 additions & 2 deletions src/components/settings/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {KeyMap} from 'utils/key-map';
import {SpeedSelectedEvent} from 'event/events/speed-selected-event';
import {actions as overlayIconActions} from 'reducers/overlay-action';
import {Tooltip} from 'components/tooltip';
import {Button} from 'components/button';

/**
* mapping state to props
Expand Down Expand Up @@ -293,13 +294,13 @@ class Settings extends Component {
return (
<div ref={c => (this._controlSettingsElement = c)} className={[style.controlButtonContainer, style.controlSettings].join(' ')}>
<Tooltip label={props.buttonLabel}>
<button
<Button
tabIndex="0"
aria-label={props.buttonLabel}
className={this.state.smartContainerOpen ? [style.controlButton, style.active].join(' ') : style.controlButton}
onClick={() => this.onControlButtonClick()}>
<Icon type={IconType.Settings} />
</button>
</Button>
</Tooltip>
{!this.state.smartContainerOpen ? (
''
Expand Down
9 changes: 5 additions & 4 deletions src/components/share-overlay/share-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {CopyButton} from '../copy-button/copy-button';
import {withLogger} from 'components/logger';
import {withKeyboardA11y} from 'utils/popup-keyboard-accessibility';
import {KeyMap} from 'utils/key-map';
import {Button} from 'components/button';

/**
* mapping state to props
Expand Down Expand Up @@ -53,7 +54,7 @@ const ShareButton = (props: Object): React$Element<any> => {
};

return (
<button
<Button
ref={el => {
props.addAccessibleChild(el);
}}
Expand All @@ -63,7 +64,7 @@ const ShareButton = (props: Object): React$Element<any> => {
className={[style.btnRounded, style[props.config.iconType], props.config.iconType].join(' ')}
onClick={() => share()}>
<Icon style={props.config.iconType === 'svg' ? `background-image: url(${props.config.svg})` : ``} type={props.config.iconType} />
</button>
</Button>
);
};

Expand Down Expand Up @@ -339,7 +340,7 @@ class ShareOverlay extends Component {
</a>
</Localizer>
<Localizer>
<button
<Button
aria-haspopup="true"
ref={el => {
this.props.addAccessibleChild(el);
Expand All @@ -348,7 +349,7 @@ class ShareOverlay extends Component {
onClick={() => this._transitionToState(shareOverlayView.EmbedOptions)}
title={<Text id="share.embed" />}>
<Icon type={IconType.Embed} />
</button>
</Button>
</Localizer>
</div>
<div className={style.linkOptionsContainer}>
Expand Down

0 comments on commit 3982da6

Please sign in to comment.