New: Add forward scrubbing prevention (fix #323) #334
Conversation
js/mediaView.js
Outdated
| scrubBlocker.classList.add('mejs__time-slider-blocker-error'); | ||
| setTimeout(() => { | ||
| scrubBlocker.classList.remove('mejs__time-slider-blocker-error'); | ||
| }, 150); | ||
| }; |
There was a problem hiding this comment.
This identical code appears three times in this file. Suggest making it a function and calling that each time.
There was a problem hiding this comment.
Also, I'm wondering about the accessibility of the 150ms red flash. If someone clicks or key presses multiple times, the red can flash in quick succession. Maybe this should only flash a maximum of once every 1 second?
There was a problem hiding this comment.
This identical code appears three times in this file. Suggest making it a function and calling that each time.
Done. Good spot.
Also, I'm wondering about the accessibility of the 150ms red flash. If someone clicks or key presses multiple times, the red can flash in quick succession. Maybe this should only flash a maximum of once every 1 second?
I'm not so sure about this call. It's not a continuous animation and the user would need to rapid-fire click to create a violation of the rule, which is highly unlikely. Also, the goal of the rule is to avoid triggering seizures. The size of the square doesn't seem large enough to fall under WCAG 2.3.1? Happy to implement something but I'm no so sure it is needed. I could be swayed the other direction pretty easily though.
|
Noting here that this PR has been extensively tested and is currently a part of a large production release with no issues. |
| .mejs__time-slider-blocker { | ||
| position: absolute; | ||
| top: 0; | ||
| right: 0; | ||
| height: 100%; | ||
| width: 0; | ||
| background-color: transparent; | ||
| pointer-events: auto; | ||
| z-index: 9999; | ||
| cursor: not-allowed; | ||
| transition: background-color 0.15s ease; | ||
|
|
||
| &.mejs__time-slider-blocker-error { | ||
| background-color: fade(@validation-error, 30%); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Will you move these styles to mep-overrides.less? The mediaelementplayer.less file is a direct copy of the library's Less file and should not be modified. Otherwise, the styles may be overridden during a library update.
cahirodoherty-learningpool
assigned
cahirodoherty-learningpool and
joe-replin and unassigned
cahirodoherty-learningpool
3 participants
#320 - was not a comprehensive solution for preventing forward scrubbing. Making another attempt.
Fix
New
Testing