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

getInheritedTheme issues #5113

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

getInheritedTheme issues #5113

jaspermdegroot opened this issue Sep 30, 2012 · 9 comments
Assignees
Milestone

Comments

@jaspermdegroot
Copy link
Contributor

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.

@ghost ghost assigned jaspermdegroot Oct 2, 2012
@gabrielschulhof
Copy link

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.

@gabrielschulhof
Copy link

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.

@scottjehl
Copy link

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.

@toddparker
Copy link
Contributor

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.

@gabrielschulhof
Copy link

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

@gabrielschulhof
Copy link

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

@jaspermdegroot
Copy link
Contributor Author

@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

@jaspermdegroot
Copy link
Contributor Author

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

@ghost ghost assigned jaspermdegroot May 2, 2013
@jaspermdegroot
Copy link
Contributor Author

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

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

No branches or pull requests

4 participants