Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken library compatibility #259

Closed
slavafomin opened this issue Nov 18, 2014 · 12 comments
Closed

Broken library compatibility #259

slavafomin opened this issue Nov 18, 2014 · 12 comments

Comments

@slavafomin
Copy link

Hi!

It looks like after one of the updates the backward compatibility was broken.

I were using library from this path:
bower/jplayer/jquery.jplayer/jquery.jplayer.js

and now it's suddenly in here:
bower/jplayer/src/javascript/jplayer/jquery.jplayer.js.

According to SemVer, when the library API is changed the major version number must be increased.

This problem can potentially break a lot of projects out there after a routine dependency update.

@thepag
Copy link
Contributor

thepag commented Nov 18, 2014

The built files are in
js/jplayer/

You're looking at the source, which is why the SWF is missing. I'll look into this as others are reporting a problem.

Apparently, I need to provide the source... which I do... But apparently, people want me to put a 100% duplicate of the JavaScript source into another folder.

@slavafomin
Copy link
Author

The convention is to put in the distribution directory both src.js and src.min.js. That way we can use unminified file during development and debugging and minified file in the production. In my case, I'm dynamically requiring unminified code from other libraries into my own single file and then I minify it during deployment.

@thepag
Copy link
Contributor

thepag commented Nov 18, 2014

OK, I will add that to the build process. And I did everything to avoid duplication of files.

@slavafomin
Copy link
Author

Thanks )

And I would recommend to rename js to dist. dist is a more common name for distribution directory, it will be easier for other developers to understand. Also, your distribution directory contains not only js-files, but also swf, so js name is not strictly correct from this perspective.

Another issue is that you should not add third-party code directly to your project's source code. I'm talking about the lib directory. You should better specify all requirements in the bower.json and install them by calling bower install. This will really help with code duplication and dependency management.

And if you really want to make the distribution package as small as possible - you can create separate repository. Please see example here. Although, I think it's an overkill.

If you need some help with organizing the repository and build process - please let me know. I'll be glad to help. I'm having some luck with Gulp recently = )

@thepag
Copy link
Contributor

thepag commented Nov 18, 2014

I'm happy to improve the organising of the repo. I spent the last 3 months doing it behind the scenes... And the repo is closely tied to the website demonstrating it all... The whole point is so that I can maintain the whole lot easily and quickly.

Problem is I'm coming to the end of my time that I can dedicate to this.

@thepag
Copy link
Contributor

thepag commented Nov 18, 2014

I will look into this tomorrow and #258

@slavafomin
Copy link
Author

It looks like CSS styling were also changed. At least player in my project is not rendered correctly. I'm using the following template:

<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"><i class="fa fa-play"></i></a></li>
                <li><a href="javascript:;" class="jp-pause" tabindex="1"><i class="fa fa-pause"></i></a></li>
                <li><a href="javascript:;" class="jp-stop" tabindex="1"><i class="fa fa-stop"></i></a></li>
                <li><a href="javascript:;" class="jp-mute" tabindex="1" title="Выключить звук"><i class="fa fa-volume-off"></i></a></li>
                <li><a href="javascript:;" class="jp-unmute" tabindex="1" title="Включить звук"><i class="fa fa-volume-off"></i></a></li>
                <li><a href="javascript:;" class="jp-volume-max" tabindex="1" title="Максимальная громкость"><i class="fa fa-volume-up"></i></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="Включить повтор"><i class="fa fa-repeat"></i></a></li>
                    <li><a href="javascript:;" class="jp-repeat-off" tabindex="1" title="Выключить повтор"><i class="fa fa-repeat"></i></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>

The proper solution would be to create a new library release (2.8.2) and remove all backward-compatibility issues in it. And then release version 3.0.0 with all the latest features. More details can be found in documentation for the SemVer: http://semver.org/.


Regarding the reorganization issue, if you'd like I can checkout development branch and do some refactoring there and create a PR after that.

@thepag
Copy link
Contributor

thepag commented Nov 18, 2014

The skins changed, yes. Backwards compatibility... The JavaScript has it. The bower and other package system might need fixing. The JavaScript is backward compatible. The skins provided are optimised for ARIA and have changed a bit. You are probably missing the useStateClassSkin:true option. The defaults are picked for backward compatability, but the skins are designed for the state class skin type, which is not the default... Since that option would break backward compatibility.

@thepag
Copy link
Contributor

thepag commented Nov 19, 2014

Gah... fixed this about an hour ago and then some idiot tried to redo it all and correct the commit message to link to this issue... And now I wish I had never bothered LOL.
Note to self, commit cock-ups in a local refactor branch before deleting it!

@thepag
Copy link
Contributor

thepag commented Nov 19, 2014

A few things on my mind now... While on the subject of these package description files:

  • The skin templates are now all provided in mustache templates, see the mustache folder inside the skin's folder.
  • How can I indicate the mustache templates in bower.json and package.json
  • Are the main and files properties correct (useful?) in the package.json
  • The bower.json file is would benefit from more information, such as the description from the package.json
  • The jplayer.jquery.json is now obsolete since plugins.jquery.com no longer updates. Remove the file from repo and the github hooks linked to the the jquery site?

@thepag
Copy link
Contributor

thepag commented Nov 19, 2014

@slavafomin I have now added a lot more information to the bower file. You should be able to review the changes with the commit links above, or through the v2.8 branch (for the time being till I merge with master and dev).

The spec I used was bower.json specification and I was able to simply use most of the data already prepared for the package.json, with the exceptions:

  • The author.url is author.homepage in bower
  • Reduced the description to less than 140 chars. Changed the package.json to match bower.json.
  • Added an ignore list.

I will go a step or two further later tonight and review using bower in practice with the jPlayer repo, and maybe try browserfy if I get along with bower. But your input and feedback is appreciated as always.

@thepag
Copy link
Contributor

thepag commented Nov 25, 2014

I'm closing this issue. Actually, I'll add the CSS and useStateClassSkin option info to the migration page, then close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants