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

Border attribute is not taken into account when calculating height #8

Closed
thijs-qv opened this issue Nov 14, 2019 · 13 comments
Closed

Comments

@thijs-qv
Copy link

If you have a border attribute on the table, eg.
<table border="1">...</table> it is not taken into account when calculating height (using autoheight: true).

@maicolsantos
Copy link

maicolsantos commented Nov 19, 2019

the following code snippet works to me, for now.
I'm waiting for an official fix.

alterChildren={(node) => {
  if (node.name === 'table') {
    node.attribs._rawHtml = node.attribs._rawHtml.replace(/border="\d+"/g, '')
  }
}

@jsamr
Copy link
Collaborator

jsamr commented Nov 19, 2019

@thijs-qv @maicolsantos This is likely a box-sizing issue.

Can you try to add a CSS rule for the table element as such, and tell me if the issue disappear?

table {
  box-sizing: border-box;
}

@thijs-qv
Copy link
Author

I've added the rule to css-rules.js, but the issue still persists.

@jsamr
Copy link
Collaborator

jsamr commented Nov 20, 2019

OK. Please provide me a minimal reproducible example (ideally, a git), and I'll be happy to investigate.

@thijs-qv
Copy link
Author

I've placed a simple example here.
Would be great if you can take a look.

@jsamr
Copy link
Collaborator

jsamr commented Nov 20, 2019

@thijs-qv @maicolsantos This is my understanding of the issue so far:

  1. The border table attribute is a legacy from old HTML specs, and is not part of the HTML 5 standard anymore.
  2. The border table attribute is ignored when my script computes table's height because it uses, for best browser support, the scrollHeight.

I could probably just add the style's borderWidth value to the scrollHeight and it should fix the problem, if and only if browser engines transfer the legacy border property to the style object, which you will have to confirm.

I will publish pre-release patches that you can test, until the issue is resolved. Will keep you updated.

@thijs-qv
Copy link
Author

Thanks for looking into it. I'm not sure if adding the borderWidth to the scrollHeight will solve the issue. The border attribute specifies if a border should be displayed around the table cells or not. So it's not just the table that gets a border, but the cells as well.

@jsamr
Copy link
Collaborator

jsamr commented Nov 20, 2019

@thijs-qv I don't think that is an issue because inner borders will be taken into account in scrollHeight. It did work on your MWE. Please test again with 0.5.1-rc.0, and let me know :-)

@thijs-qv
Copy link
Author

@jsamr You are correct, that is not an issue, it's working well now. Thank you / merci beaucoup!

@jsamr jsamr closed this as completed in 5fe4a94 Nov 20, 2019
@jsamr
Copy link
Collaborator

jsamr commented Nov 20, 2019

@thijs-qv You're very welcomed!

@maicolsantos
Copy link

@jsamr it's working well now. Thank you 🤓

@jsamr
Copy link
Collaborator

jsamr commented Nov 20, 2019

@maicolsantos I realize you're from Fortaleza, aren't you? I've just moved from France to Juazeiro do Norte, what a coincidence!

@maicolsantos
Copy link

@jsamr I sent you an email. 🤓

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

3 participants