Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Bootstrap themes no longer apply to ui-bootstrap elements #5254

Closed
nickdnk opened this issue Jan 14, 2016 · 16 comments
Closed

Bootstrap themes no longer apply to ui-bootstrap elements #5254

nickdnk opened this issue Jan 14, 2016 · 16 comments

Comments

@nickdnk
Copy link

nickdnk commented Jan 14, 2016

Hello

Up until 1.0 I could use something like Bootswatch to override the general theme on my page. If I upgrade to 1.0, the the uib templates have inline styling that override my CSS and hence adopt the default bootstrap theme.

Is this intended behavior? I cannot find anything about it in the documentation for breaking changes.

Edit: Unless it's related to this: "All virtual templates in UI Bootstrap now are prefixed with uib/ - if one is overriding the templates via $templateCache path or manually building the templates from the UI Bootstrap repository, one needs to change the string used in the $templateCache representation to have the new prefix". I'm not sure though?

@icfantv
Copy link
Contributor

icfantv commented Jan 14, 2016

@nickdnk, I'm hearing two different things: CSS styles and template paths. Can you confirm? Thanks.

@nickdnk
Copy link
Author

nickdnk commented Jan 14, 2016

@icfantv Sorry - not so experienced with how templates work. Let me elaborate.

If I use a <uib-tabset> for instance. In <1.0, the styling will follow if I apply a theme from bootswatch.com. In >1.0, the theme will not be applied to the tabset. If I inspect the element in >1.0, the element has inline styling that overrides what's in my bootswatch CSS.

@icfantv
Copy link
Contributor

icfantv commented Jan 14, 2016

Ah, yes. That was my first thought - tabs. Due to #5193 and a few other issues - we had to change the template to use <div> elements instead of <a>. We didn't realize the CSS ramifications this would have so we added the CSS in 1.0.3. Note that we also have an outstanding PR to add in the necessary CSS for justified tabs that has not yet been committed (#5245).

I think this should fix your issues?

@nickdnk
Copy link
Author

nickdnk commented Jan 14, 2016

Looks familiar. So I'll just hold out until 1.0.4 and see if that fixes it, yes? :)

@icfantv
Copy link
Contributor

icfantv commented Jan 14, 2016

That's only relevant if you're using justified tabs. If not, that PR isn't going to do anything for you.

I took a look. The paths into the $templateCache did change. It went from template/tabs/tab.html to uib/template/tabs/tab.html in 1.0.

@nickdnk
Copy link
Author

nickdnk commented Jan 14, 2016

Okay. Not sure if you're indicating that I should do something, that Bootswatch should do something or that you will do something? This is little beyond the scope of problems I normally deal with in my development process. CSS and styling in general. My tabs are not justified.

@icfantv
Copy link
Contributor

icfantv commented Jan 14, 2016

Oooooooooooh. I get it now. I didn't know what Bootswatch was and had to look. So yea, we're using the default colors from Bootstrap CSS so if you need/use a different theme, you will need to override that on your end. I realize this stinks and am sorry. If bootstrap would change their tabs from anchors to divs then we could revert all this (but then they'd have thousands of people screaming as well...).

Does this answer your question?

@rm6
Copy link

rm6 commented Jan 14, 2016

Guys, I have the same issue with a custom theme. I think I understand what is going on here, someone tried to create a tab with a button in it. Instead of recommending a e.preventDefault() as a workaround, we moved away from bootstrap styles and created a div with custom styling mimicking bootstrap tabs and stuffed it into the DOM as local styles. I now cannot even override the styles for the fake div in my CSS since the styles in the dom have higher precedence (unless i override all of the DOM styles with a nasty !important)

@rm6
Copy link

rm6 commented Jan 14, 2016

I am suspecting many people with any kind of custom bootstrap styling is facing the same issue.

@nickdnk
Copy link
Author

nickdnk commented Jan 14, 2016

That's too bad. Especially since Bootswatch feels no responsibility towards angular-ui. No common ground of interest to fix it, it seems. :(

Sent from my iPhone

On 14 Jan 2016, at 21:52, Adam Gordon <[email protected]mailto:[email protected]> wrote:

Oooooooooooh. I get it now. I didn't know what Bootswatch was and had to look. So yea, we're using the default colors from Bootstrap CSS so if you need/use a different theme, you will need to override that on your end. I realize this stinks and am sorry. If bootstrap would change their tabs from anchors to divs then we could revert all this (but then they'd have thousands of people screaming as well...).

Does this answer your question?

Reply to this email directly or view it on GitHubhttps://github.com//issues/5254#issuecomment-171775940.

@icfantv
Copy link
Contributor

icfantv commented Jan 14, 2016

We've not heard a ton of complaints yet about the hard-coding of styles but I suspect that will come as people upgrade.

@rm6 - it wasn't just a button in a tab header (which I would agree, is probably not the best UX), it was the browser interpreting the click on the anchor tag as a route navigation/page request. If you look at the Angular code for the href directive, you will see the event.stopPropagation call, but the problem was that the directive was never even getting fired - that is, the browser was acting on the anchor click before Angular could even react.

Hope that was more clear. Time will tell if this was a decision that will work. Do know that we discussed this for a LONG time before making the tough decision.

@rm6
Copy link

rm6 commented Jan 14, 2016

What do you think about adding some kind of an option to disable the page level internal styling so people could style them according to their custom bootstrap theming ? internal style elements directly on the page is pretty disruptive for custom external CSS.

@icfantv
Copy link
Contributor

icfantv commented Jan 14, 2016

I'm ok w/ that so long as it's turned on by default and there's actually a way to disable it at runtime and not angular boot time. @wesleycho, @Foxandxss, what do you guys think? Is this doable? If so, I would say that if the user chooses that option, we need to be clear about what we would support and what we would not support.

@nickdnk
Copy link
Author

nickdnk commented Jan 14, 2016

I agree that this would probably be a good thing to solve. I think a lot of other developers will suffer from this incompatibility in the near-future, as they upgrade. As of right now I'm stuck at 0.14.3.

Sent from my iPhone

On 14 Jan 2016, at 23:56, Adam Gordon <[email protected]mailto:[email protected]> wrote:

I'm ok w/ that so long as it's turned on by default and there's actually a way to disable it at runtime and not angular boot time. @wesleychohttps://github.com/wesleycho, @Foxandxsshttps://github.com/Foxandxss, what do you guys think? Is this doable? If so, I would say that if the user chooses that option, we need to be clear about what we would support and what we would not support.

Reply to this email directly or view it on GitHubhttps://github.com//issues/5254#issuecomment-171807998.

@wesleycho
Copy link
Contributor

Hmm, I would be fine with configuration to handle this - this is a really crappy situation, all stemming from Bootstrap getting this wrong, but we don't have control over that.

Maybe reverting all of those changes to tabs to use anchor tags and mentioning as a known issue that if one wants custom clickable elements inside the tab, one should override the template to use div elements.

I don't like this situation, but there is little we can do here. Perhaps even this attempt to fix broken behavior is not worth it given the pain to custom themes....although there is going to be this pain in the future once we remove replace: true usage from the library.

@wesleycho
Copy link
Contributor

PRs are welcome if anyone wants to attempt the reverting back to anchor tags behavior, the documentation for the specific situation I mentioned, I can do.

@wesleycho wesleycho added this to the 1.0.4 milestone Jan 15, 2016
wesleycho added a commit to wesleycho/bootstrap that referenced this issue Jan 15, 2016
- Reverts template change to tab
- Adds warning if one desires more complex HTML inside the tab

BREAKING CHANGE: This undoes the prior change to the template using div
elements. If one wishes to use div elements, one must override the
template in one's app and provide the necessary CSS

Closes angular-ui#5262
Fixes angular-ui#5254
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants