Skip to content

/ jquery

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

Jump to bottom

$.show() can break responsive stylesheets #1767

Closed
mgol opened this issue Oct 21, 2014 · 4 comments
Closed

$.show() can break responsive stylesheets #1767

mgol opened this issue Oct 21, 2014 · 4 comments
Assignees
@gibson042
Labels
Blocker CSS
Milestone
3.0.0

Comments

@mgol
Copy link
Member

@mgol mgol commented Oct 21, 2014

Originally reported by sten_paa@… at: http://bugs.jquery.com/ticket/15037

When $.show() is called the computed display value is cemented as an inline style. This can cause inconsistent behavior.

Fiddle:  http://jsfiddle.net/jinglesthula/ZttLJ/

Note that in the fiddle I use JavaScript to change the 'layout'. This simulates what would happen if the display: inline style were applied conditionally based on a responsive media query. That is, a given block of code could produce different results based on something like the orientation of a device and when the device orientation (really, the viewport width) changed with respect to when the show/hide calls are made.

I understand that based on the fact that an inline style is applied at the point when .show() is called that the browser is rendering the styles properly. The 'bug' I suppose comes in the fact that a dynamic computed value is assumed to be ok to translate to a more fixed inline style.

Resolution may involve finding a way to 'clean up' the inline style at the end of .show() and allow all styles not provided by jQuery to continue doing exactly what they were before show/hide were invoked.

Issue reported for jQuery 1.11.0
@mgol mgol added this to the 3.0.0 milestone Oct 21, 2014
@mgol
Copy link
Member Author

@mgol mgol commented Oct 21, 2014

Comment author: gibson042

Cleanup is not possible in the current API because (for better or worse) .show() works on elements suppressed by stylesheet rules. This may be an issue best solved by plugin, but it does seem like we've taken the wrong path (which has also affected us through defaultDisplay). I'll leave this for 1.12/2.2 consideration.

@mgol
Copy link
Member Author

@mgol mgol commented Oct 21, 2014

Comment author: dmethvin

Let's address this via docs, because I don't think there is any way we can read all possible intents with .show() and .hide(). In general I'd like to deprecate them and encourage people to use classes instead. There may be some code changes to also be made here (*simplifications* I would hope) so I'll leave the ticket open.

@mgol
Copy link
Member Author

@mgol mgol commented Oct 21, 2014

Comment author: gibson042

It was noted in the meeting today that ignoring stylesheets and just looking at inline styles would resolve this, #15192, #13962, #9065, and a host of other show/hide/toggle bugs... and probably eliminate defaultDisplay to boot. Of course, there's no way to know about bugs that would have been filed with different behavior, but it's worth looking into the consequences of such a breaking change.

@gibson042 gibson042 self-assigned this Dec 7, 2014
@timmywil timmywil added the CSS label Feb 1, 2015
@timmywil timmywil mentioned this issue Feb 1, 2015
Closed
@markelog markelog mentioned this issue Feb 3, 2015
Closed
@chriscct7 chriscct7 mentioned this issue Feb 9, 2015
Closed
@timmywil timmywil added the Blocker label Feb 10, 2015
gibson042 added a commit to gibson042/jquery that referenced this issue Mar 30, 2015
@gibson042
CSS: Ignore the CSS cascade in show()/hide()/etc.
4ae9095 
WIP (failing effects tests)

Fixes jquerygh-1767
@gibson042 gibson042 mentioned this issue Mar 30, 2015
Closed
gibson042 added a commit to gibson042/jquery that referenced this issue Apr 5, 2015
@gibson042
CSS: Ignore the CSS cascade in show()/hide()/etc.
6e7186f 
WIP (failing effects tests)

Fixes jquerygh-1767
gibson042 added a commit to gibson042/jquery that referenced this issue Apr 6, 2015
@gibson042
CSS: Ignore the CSS cascade in show()/hide()/etc.
38f8819 
WIP (failing effects tests)

Fixes jquerygh-1767
gibson042 added a commit that referenced this issue May 11, 2015
@gibson042
CSS: Ignore the CSS cascade in show()/hide()/etc.
Loading status checks…
9df8bd2 
Fixes gh-1767
Fixes gh-2071
Closes gh-2180

(cherry picked from commit 86419b1)

Conflicts:
	src/css.js
	src/css/defaultDisplay.js
	src/effects.js
	test/data/testsuite.css
	test/unit/css.js
	test/unit/effects.js
@gibson042 gibson042 closed this in 86419b1 May 11, 2015
@timmywil timmywil mentioned this issue Jun 2, 2015
Closed
@markelog markelog mentioned this issue Nov 16, 2015
Closed
@markelog markelog mentioned this issue Dec 22, 2015
Closed
@gibson042 gibson042 mentioned this issue Jan 20, 2016
Closed
@sqrthree sqrthree mentioned this issue Feb 20, 2016
Open
@jinglesthula
Copy link

@jinglesthula jinglesthula commented Jul 27, 2016

On this page http://api.jquery.com/show/ a yellow Note callout was added indicating show can cause issues with responsive layout. Now that 3.0 has been released and fixes the issue* I'm wondering if the note should be updated to indicate that <3.0 has the responsive issue, unless there are still other responsive layout issues with show even in 3.0.

* at least in the fiddle above it fixed it when I switched it to 3.0 - I didn't do other testing regarding responsive layout + show/hide.
@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@gibson042 gibson042

Labels
Blocker CSS
Projects
None yet
Milestone
3.0.0
Linked pull requests

Successfully merging a pull request may close this issue.

CSS: Ignore the CSS cascade in show()/hide()/etc.
5 participants
@dmethvin @timmywil @jinglesthula @gibson042 @mgol