getInheritedTheme issues #5113

Closed
jaspermdegroot opened this Issue Sep 30, 2012 · 9 comments

Comments

Projects
None yet
4 participants
Member

jaspermdegroot commented Sep 30, 2012

There are several issues with getInheritedTheme.

Test page v1: http://jsbin.com/ezakuk/8

  • issue 1: We changed static list items for 1.2. They now use class ui-btn-up- without background-image instead of ui-body-. Because the getInheritedTheme regex only looks for bar|body|overlay, buttons and form elements don't inherit the swatch from the list item. This can be fixed by including ui-btn-up- in the regex.

Test page v2 which uses a build that includes the fix for issue 1: http://jsbin.com/ezakuk/11

  • issue 2: Buttons that only use the buttonMarkup function and not the button widget (e.g. anchors with data-role="button") don't inherit the swatch from their container. However, this issue only appears in a build. If you use http://localhost/[path]/js/ as source you won't see the issue.
    So it looks like this has to do with the differences between js/index.php and js/jquery.mobile.js.
  • issue 3: In a popup "issue 2" also applies to buttons that use the button widget. This occurs local and in a build.
    With a popup it is local that shows problems that are not visible in a build; the select button and text input don't inherit the swatch from their container (screenshot).

I tested this on a few different platforms/browsers, and with jQuery core 1.7.1 - 1.8.2, but the results were always the same. I tested "issue 2" with JQM 1.1.1 and it already existed in that version.

Contributor

gabrielschulhof commented Nov 1, 2012

I think another problem is that buttons are getting enhanced before list items. So, when getInheritedTheme when called from buttonMarkup starts looking for theme-related classes, it finds none, because they have not yet been applied.

Contributor

gabrielschulhof commented Nov 1, 2012

Here's the problem, I think: listview requires buttonMarkup, but jquery.mobile.buttonMarkup.js contains the pagecreate handler that causes buttons to become enhanced, so that handler is registered before the listview pagecreate handler.

I guess we can't rely on the pagecreate handlers to be in the correct order. The order in which they fire probably reflects the define-time dependencies between the widgets/plugins that register the events, not the runtime dependency they have on other widgets present in the DOM.

Contributor

scottjehl commented Nov 1, 2012

This is a real problem with automated theming in general. We had similar issues in jQuery UI and it crops up here too.

I don't think we can do much to fix this in general without introducing another unexpected change. It all just depends on the order in which you embed widgets in a UI. Maybe we can do a better job of planning our order (maybe buttonMarkup.js should come later, for instance), but there are going to be some widgets that can embed in either direction.

Down the road, we should continue to decrease our reliance on JS and HTML for anything CSS can do best (even if it takes a little more manual work on end-developers' behalf to configure properly). Waiting on JS manipulation for CSS theming is one of those areas we need to change, I think.

Contributor

toddparker commented Nov 1, 2012

We need to re-consider the complexity of theme inhertitance and may move away from this at some point in the future so de-prioritizing these fixes.

Contributor

gabrielschulhof commented Nov 1, 2012

Here's an example that won't look right, no matter the order in which pagecreate handlers fire: http://jsbin.com/uzaret/432

Contributor

gabrielschulhof commented Nov 5, 2012

Updated the example to be a bit more informative: http://jsbin.com/uzaret/452

Member

jaspermdegroot commented Nov 18, 2012

@gabrielschulhof

After commit c32d94b we have another getInheritedTheme issue. The dialog close button doesn't inherit the theme from the header anymore.

Test page: http://jsbin.com/omegup/29

Long custom select (second example, "Your state:"): http://jquerymobile.com/test/docs/forms/selects/custom.html

Member

jaspermdegroot commented Feb 3, 2013

Issue with dialogs as metioned in previous comment has been resolved.
New issue: theme inheritance doesn't work in panels (at least not local).

Member

jaspermdegroot commented Jun 13, 2013

Closing as fixed by commit 4973827. Theme inheritance is done with CSS. The getInheritedTheme function has been removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment