search

jplayer / jPlayer

jPlayer : HTML5 Audio & Video for jQuery
http://jplayer.org/
Other
4.6k stars 1.47k forks source link

Better HTML semantics #232

Closed slavafomin closed 10 years ago

slavafomin commented 10 years ago

Hello and thanks for this great library!

I would like to suggest to improve the semantics of the player's HTML.

Here's the code from documentation:

<div id="jquery_jplayer_1" class="jp-jplayer"></div>
<div id="jp_container_1" class="jp-audio">
    <div class="jp-type-single">
        <div class="jp-gui jp-interface">
            <ul class="jp-controls">
                <li><a href="javascript:;" class="jp-play" tabindex="1">play</a></li>
                <li><a href="javascript:;" class="jp-pause" tabindex="1">pause</a></li>
                <li><a href="javascript:;" class="jp-stop" tabindex="1">stop</a></li>
                <li><a href="javascript:;" class="jp-mute" tabindex="1" title="mute">mute</a></li>
                <li><a href="javascript:;" class="jp-unmute" tabindex="1" title="unmute">unmute</a></li>
                <li><a href="javascript:;" class="jp-volume-max" tabindex="1" title="max volume">max volume</a></li>
            </ul>
            <div class="jp-progress">
                <div class="jp-seek-bar">
                    <div class="jp-play-bar"></div>
                </div>
            </div>
            <div class="jp-volume-bar">
                <div class="jp-volume-bar-value"></div>
            </div>
            <div class="jp-time-holder">
                <div class="jp-current-time"></div>
                <div class="jp-duration"></div>
                <ul class="jp-toggles">
                    <li><a href="javascript:;" class="jp-repeat" tabindex="1" title="repeat">repeat</a></li>
                    <li><a href="javascript:;" class="jp-repeat-off" tabindex="1" title="repeat off">repeat off</a></li>
                </ul>
            </div>
        </div>
        <div class="jp-details">
            <ul>
                <li><span class="jp-title"></span></li>
            </ul>
        </div>
        <div class="jp-no-solution">
            <span>Update Required</span>
            To play the media you will need to either update your browser to a recent version or update your <a href="http://get.adobe.com/flashplayer/" target="_blank">Flash plugin</a>.
        </div>
    </div>
</div>

It uses A tags for buttons: <a href="javascript:;" class="jp-play" tabindex="1">play</a> which is semantically incorrect. Why not use the button element instead?

It would look like this: <button class="jp-play" tabindex="1">play</button>. The code will actually be shorter as we do not need an extraneous href="javascript:;" hack.

Cheers! = )

thepag commented 10 years ago

Sorry for the late reply... By coincidence, I've been doing this for the ARIA changes in the dev branch. At time of writing, the Pink Flag skin is pretty much there.