Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Navbar: Implement Classes Option #8297

Closed
wants to merge 34 commits into from

Conversation

cgack
Copy link
Contributor

@cgack cgack commented Sep 30, 2015

This includes the new navbar widget from the navbar review branch.

@jquerybot
Copy link

Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA).

📝 Please visit http://contribute.jquery.org/CLA/ to sign.

After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know.


If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check.

@cgack
Copy link
Contributor Author

cgack commented Sep 30, 2015

@arschmitz ready for review when you get a chance

if ( $.mobileBackcompat !== false ) {
return $.widget( "mobile.navbar", $.mobile.navbar, {
_create: function() {
var self = this;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use self this break in some contexts self has special meaning window.self === window instead use var that = this;

@cgack
Copy link
Contributor Author

cgack commented Oct 16, 2015

@arschmitz I made some changes. ready for another round when you get a chance

@dt1973
Copy link

dt1973 commented Oct 16, 2015

Sorry to interject, but will this prevent jQM from inserting classes into my navbar elements such as h2? I'm using the structure only CSS, and right now I have to wrap all my stuff inside <div></div> to prevent this from happening.

@arschmitz
Copy link
Contributor

@dt1973 can you open an issue explaining your issue with a jsbin example and we will be happy to take a look and consider this case.

that.navButtons = navButtons;
that.numButtons = numButtons;
that.maxButton = maxButton;
that.iconpos = iconpos;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use this

fix styling for more button

Fixes jquery-archivegh-7705
merge with latest toolbar

Fixes jquery-archivegh-7705
added backcompat tests and classes

Fixes jquery-archivegh-7705
hasClasses and lacksClasses in tests

Fixes jquery-archivegh-7705
remove deprecated jqmEnhanceable

Fixes jquery-archivegh-7705
morebutton should be a button

Fixes jquery-archivegh-7705
cgack added a commit that referenced this pull request Oct 29, 2015
@cgack
Copy link
Contributor Author

cgack commented Oct 29, 2015

Closing with f3bd7c4 landing on 1.5-dev

@cgack cgack closed this Oct 29, 2015
arschmitz pushed a commit to arschmitz/jquery-mobile that referenced this pull request Jul 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants