Skip to content
This repository has been archived by the owner. It is now read-only.

Panel: Implement classes option #8350

Closed

Conversation

apsdehal
Copy link
Contributor

@apsdehal apsdehal commented Dec 22, 2015

Fixes #7709

pageFixedToolbar: "ui-panel-fixed-toolbar",
pageContentPrefix: "ui-panel-page-content", /* Used for wrapper and fixed toolbars position, display and open classes. */
animate: "ui-panel-animate"
"ui-panel": "ui-panel-closed"

Choose a reason for hiding this comment

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

@arschmitz can correct me if I'm wrong, but I don't believe ui-panel-closed is supposed to be a value, but rather a key. The value of a class key is supposed to be a set of style-related classes, IINM.

Copy link
Contributor Author

@apsdehal apsdehal Dec 22, 2015

Choose a reason for hiding this comment

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

I suppose then there is not even a need of classes option here.


// if animating, add the class to do so
this._addClass( this.element, "ui-panel ui-panel-closed", this._getPanelClasses() );
Copy link
Contributor

@arschmitz arschmitz Jan 28, 2016

Choose a reason for hiding this comment

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

no need to pass this.element its the default, this._addClass( "ui-panel ui-panel-closed", this._getPanelClasses() );

@arschmitz
Copy link
Contributor

arschmitz commented Feb 18, 2016

this looks good once you make the 2 changes

@apsdehal
Copy link
Contributor Author

apsdehal commented Feb 18, 2016

@arschmitz Done!

@arschmitz
Copy link
Contributor

arschmitz commented Feb 25, 2016

👍

apsdehal added a commit that referenced this pull request Feb 25, 2016
@apsdehal apsdehal closed this Feb 25, 2016
apsdehal added a commit to apsdehal/jquery-mobile that referenced this pull request Mar 26, 2016
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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants