Skip to content
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

Style Table Element #99

Closed
Eusse opened this issue Jul 13, 2016 · 14 comments
Closed

Style Table Element #99

Eusse opened this issue Jul 13, 2016 · 14 comments

Comments

@Eusse
Copy link

Eusse commented Jul 13, 2016

Hi, i'm trying to style the table element with bootstrap like this. I added the classNames attribute in the component template but it adds the classes to the div, not the table tag.
{{#light-table table classNames='table table-bordered table-hover table-condensed' as |t|}}
Is this supported? am i doing something wrong?

ember-light-table: 1.0.0
ember-cli: 2.6.2
node: 6.2.2
os: linux x64

@offirgolan
Copy link
Collaborator

Im not able to recreate your issue. Using classNames should work... Im using it in my apps with no problems.

{{#light-table table classNames="my-custom-class" height='65vh' as |t|}}
 ...
{{/light-table}}

screen shot 2016-07-19 at 11 24 15 am

@Eusse
Copy link
Author

Eusse commented Jul 19, 2016

Yeah, that's the issue. I need the classes to be applied to the table tag, not the div, because if it's applied to the div, the cells wont get the borders.
ember-table-bootstrap

@offirgolan
Copy link
Collaborator

@Eusse can you target your CSS at the table tag? Instead of .table-bordered have it be .table-bordered table

@mwalper
Copy link

mwalper commented Jul 21, 2016

That's what i did to fix this, but i'm not sure that's the right way to do it.
You can place whichever css class(es) you want.
In Parent Component:

didInsertElement() {
    "use strict";
    Ember.run.next(this, function(){
        this.$('table').addClass('table m-b-0 table-bordered table-hover table-condensed');
    });
},

@Eusse
Copy link
Author

Eusse commented Jul 21, 2016

@offirgolan Not really, i need to use default bootstrap css and it says i should use all the classes within table tag as mentioned in the docs link. @mwalper I will try this, but i would like to have some way to specify it.

@buschtoens
Copy link
Collaborator

@Eusse right now, this is the only way, besides overriding the template, which I would discourage.

Maybe we should implement a tableClassNames property or something similar and add it to the <table> tag (also in lt-head and lt-footer).

@buschtoens
Copy link
Collaborator

Side note: applying the classes on the wrapping <div> doesn't work because of Bootstrap using direct descendant selectors in their tables.less. They surely got their reasons, but I heavily dislike these kinds of overly restrictive CSS rules. But I got good news for you: They've fixed it in Bootstrap v4. 😄

@Eusse
Copy link
Author

Eusse commented Jul 22, 2016

That sounds great @buschtoens! but looking at the alpha 4 documentation still needs the classes in the table tag. I like better your idea of implementing the TableClassNames property, besides, other frameworks like zurb foundation use this approach as well. I think we should add this feature.

@buschtoens
Copy link
Collaborator

buschtoens commented Jul 22, 2016

Disregard the documentation. It's only exemplary. 😉
The source code I linked clearly shows that they don't use the direct descendant / sibling selector in v4.

I think this is more of an issue with Bootstrap and not ember-light-table. IMHO the CSS shouldn't be so tightly coupled to the HTML structure as this leads to exactly these kinds of problems.

Ultimately, this decision is up to @offirgolan, but personally I think this unnecessarily increases the API surface area while providing little benefit. Independent of this issue, I recommend switching to v4 for new projects - is this not an option for you?

@offirgolan
Copy link
Collaborator

@buschtoens adding something like tableClassNames is very simple to do and I believe will resolve this issue. Ill take some time this week to add this feature. @taras does this solution work for you?

@taras
Copy link
Collaborator

taras commented Jul 25, 2016

@offirgolan yeah, I think tableClassNames is a good solution here. Are you thinking something like this?

{{light-table table tableClassNames="table table-bordered table-hover table-condensed"}}

@offirgolan
Copy link
Collaborator

offirgolan commented Jul 25, 2016

@taras yup exactly that. Then apply all those classes to each table being
rendered.

@Eusse
Copy link
Author

Eusse commented Jul 25, 2016

@buschtoens You are right, i tried and it works well, i can switch to v4 for my personal projects, but i cannot change my client's bootstrap version.
Thanks for the help guys!

@fran-worley
Copy link
Contributor

Sorry to bump an old issue but I believe this only partially resolves issues marrying up Bootstraps css styles with ember-light-table.

We need to be able to apply styles directly to the 'thead' too ('.thead-default' is just one style example from BS v4). Could the api be harmonized to allow you to add classes directly to table elements (table, thead, tbody etc.).

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

No branches or pull requests

6 participants