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

How to make table border=none setting work? #6841

Open
Reinmar opened this issue May 18, 2020 · 14 comments · May be fixed by #16002
Open

How to make table border=none setting work? #6841

Reinmar opened this issue May 18, 2020 · 14 comments · May be fixed by #16002
Labels
domain:dx This issue reports a developer experience problem or possible improvement. domain:ui/ux This issue reports a problem related to UI or UX. package:table squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Reinmar
Copy link
Member

Reinmar commented May 18, 2020

📝 Provide a description of the improvement

If you set table borders to none this is what you'll likely see:

The table still has a border. Why does it happen? It's because the table is styled by default content styles to this:

These styles are applied by default via a stylesheet to every table in the editor. Since the "none" style set in the "Table properties" balloon does not apply any inline style to the <table> element, the styles coming from the stylesheet are applied.

Therefore, in order to make a table not have any border by default one needs to add such a CSS to their website's styles (note: tds need to be styled too):

body .ck-content .table table, body .ck-content .table td {
    border: none;
}

With this style, you'll get:

However, this isn't perfect – the table is now rendered in a very unclear way. Therefore, it's better to load different stylesheet in a page where the editor is rendered and in a page where the editor contents is rendered:

/* page-with-editor.css */
body .ck-content .table table, body .ck-content .table td {
    border: dotted 1px #ddd;
}
/* page-with-editor-content.css */
body .ck-content .table table, body .ck-content .table td {
    border: none;
}

This will make the table visible when rendered in the editor even if it has no border set via the "Table properties" and "Table cell properties" balloons:

While ensuring that everything still works when the table is being styled via the balloons:

And that the border is not rendered at all if it was not set via the balloon when the content outputted from the editor is rendered on a target page (that uses page-with-editor-content.css).

Should we do something with this?

It may be confusing for some developers why setting borders to none via the UI does not make the borders disappear completely. It'd be good to at least point in the documentation to this ticket.

However, perhaps we should do more?


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@Reinmar Reinmar added the type:improvement This issue reports a possible enhancement of an existing feature. label May 18, 2020
@Reinmar Reinmar added this to the backlog milestone May 18, 2020
@Reinmar Reinmar added domain:dx This issue reports a developer experience problem or possible improvement. domain:ui/ux This issue reports a problem related to UI or UX. package:table labels May 18, 2020
@jswiderski
Copy link

jswiderski commented Jun 16, 2020

I think we should do more with this one. As you know there is a style border: none and when working with the table for the first time I assumed that our table properties dialog will assign inline style of border:none or border-style:none to the table, however nothing like that happened - no style was assigned.

It looks to me that "border none" in our dialog serves as default value. Something like "nothing has been selected from the dropdown" which in my opinion is invalid thus I propose adding new default option to the dropdown like e.g. "Select One" or "Default" and making the "None" setting actually applying border:none to the table. We are applying inline styles for other border styles like solid or dotted so we should also apply the style for none.

@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label Jun 16, 2020
@remigasas
Copy link

Issue #6363 was closed and this issue was created. Close reason was duplicate, but it isn't.

Our use case, that I tried to explain in closed issue, is that we want to add table with borders by default and allow users to change selected borders as none ( invisible ) as needed ( this functionality is working in CK4, we are migrating to CK5 and this is breaking deal for us ). Majority of our tables will have borders only very few (most of them are needed for layout purposes will be partially or fully invisible ). Adding tables with full borders is how other editors works, it is intuitive and users already familiar with table feature from other products.

I think for developers to start changing default css values is a big no-no. Both plugin and default css values comes from CK editor. Your are in full control of them. If those css values needs to be changed in future, it can be very complicated for projects relying on CK editor to update versions, without introducing subtle bugs. Especial if your customers rely on backwards comparability ( document created with CK version x should render same with CK version x+1 ).

Ideally table plugin should be aware of css properties ( that comes from stylesheets ) and show correct border styles. If that is too much work or CK team wants to avoid detecting css properties ( very very valid reasons ) perhaps there can be some kind of hook/method that I can provide in config, which would be called when table is added and I could apply styling there ?

Or perhaps, even a flag, which if set, would add all css properties for solid table inside html. This would increase html size, but then plugin would be in full control of table and wouldn't need to assume what are defaults in stylesheets.

Both options seems hacky, again, I think ideally plugin should be aware of css coming from styleheet.

@pomek pomek removed this from the backlog milestone Feb 21, 2022
@Reinmar Reinmar added the squad:core Issue to be handled by the Core team. label May 12, 2022
@hendisantika
Copy link

How do I use if I am using Nuxt 3 @Reinmar ?

@Witoso
Copy link
Member

Witoso commented Jan 9, 2024

@hendisantika you could override the CSS as suggested in the description.

@hendisantika
Copy link

📝 Provide a description of the improvement

If you set table borders to none this is what you'll likely see:

The table still has a border. Why does it happen? It's because the table is styled by default content styles to this:

These styles are applied by default via a stylesheet to every table in the editor. Since the "none" style set in the "Table properties" balloon does not apply any inline style to the <table> element, the styles coming from the stylesheet are applied.

Therefore, in order to make a table not have any border by default one needs to add such a CSS to their website's styles (note: tds need to be styled too):

body .ck-content .table table, body .ck-content .table td {
    border: none;
}

With this style, you'll get:

However, this isn't perfect – the table is now rendered in a very unclear way. Therefore, it's better to load different stylesheet in a page where the editor is rendered and in a page where the editor contents is rendered:

/* page-with-editor.css */
body .ck-content .table table, body .ck-content .table td {
    border: dotted 1px #ddd;
}
/* page-with-editor-content.css */
body .ck-content .table table, body .ck-content .table td {
    border: none;
}

This will make the table visible when rendered in the editor even if it has no border set via the "Table properties" and "Table cell properties" balloons:

While ensuring that everything still works when the table is being styled via the balloons:

And that the border is not rendered at all if it was not set via the balloon when the content outputted from the editor is rendered on a target page (that uses page-with-editor-content.css).

Should we do something with this?

It may be confusing for some developers why setting borders to none via the UI does not make the borders disappear completely. It'd be good to at least point in the documentation to this ticket.

However, perhaps we should do more?

If you'd like to see this improvement implemented, add a 👍 reaction to this post.

Where can I find table properties?

image

@hendisantika
Copy link

BTW, Where I can set Table Properties Balloon?
Thanks

@Witoso
Copy link
Member

Witoso commented Jan 10, 2024

@hendisantika check the docs in that matter.

@Mgsy
Copy link
Member

Mgsy commented Feb 22, 2024

It's possible to achieve border: none by configuring table and table cell default properties to match our content styles. In this case, the following configuration should be sufficient:

        table: {
            tableProperties: {
                // The default styles for tables in the editor.
                // They should be synchronized with the content styles.
                defaultProperties: {
                    borderStyle: 'double',
                    borderColor: 'hsl(0, 0%, 70%)',
                    borderWidth: '1px',
                    width: '100%',
                    height: '100%'
                }
                // The default styles for table cells in the editor.
                // They should be synchronized with the content styles.
            },
            tableCellProperties: {
                defaultProperties: {
                    borderStyle: 'solid',
                    borderColor: 'hsl(0, 0%, 75%)',
                    borderWidth: '1px'
                }
            }
        }

Inserted table:

Changed table and table cells to border:none

I believe it would make sense to set these values by default in our feature to match our content styles.

@Witoso
Copy link
Member

Witoso commented Feb 26, 2024

I believe it would make sense to set these values by default in our feature to match our content styles.

I agree, it's confusing right now, and our default config should match the styles.

@map-r
Copy link
Contributor

map-r commented Mar 6, 2024

The @Mgsy solution works fine, will push this change, but I wanted to take it even one step further. Perhaps we could also have the option to propagate the table border styles to the cell styles (as a checkbox or by default, no strong opinions on that), because changing table border style to none first doesn't visually change the table until cell borders are changed as well, which feels counterintuitive.

Let me know what do you think.

@Witoso
Copy link
Member

Witoso commented Mar 7, 2024

doesn't visually change the table until cell borders are changed as well, which feels counterintuitive.

Yes, this is the HTML environment that influences the UI. While I like the propagation idea, it's one of many. The other one is to have tables in two modes, advanced (HTML), and basic (Notion-like). I wouldn't increase the scope in this ticket, this will be in scope for the future improvements of tables.

@map-r
Copy link
Contributor

map-r commented Mar 7, 2024

One more thing to consider - while testing the above solution (setting defaultProperties in table config), I've realized that new attributes are not added into the model when they are the same as default config values. For example, if you upcast <table border:"1px double #89B">...</table>, then only tableBorderColor: '#89B' will be set on the node in the model, because 1px and double are the same as in the ⬆️ default config.
Do we want to keep this behaviour or also modify it to always have 1-1 mapping of attributes view - model?

@Witoso
Copy link
Member

Witoso commented Mar 7, 2024

I think it would be against our current default in other features: Font Size, Font Type, when we operate on default, we don't keep them (in model or HTML). But in those, we don't specifically set the values (just "default"), we just use stylesheets 🤔 .

We also got this recently: #15917

@Reinmar any context we are missing here?

@Witoso
Copy link
Member

Witoso commented Mar 7, 2024

There's also this discussion: #14921 (comment)

@map-r map-r linked a pull request Mar 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:dx This issue reports a developer experience problem or possible improvement. domain:ui/ux This issue reports a problem related to UI or UX. package:table squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants