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

try fixing #480 #513

Merged
merged 9 commits into from
Jul 12, 2019
Merged

try fixing #480 #513

merged 9 commits into from
Jul 12, 2019

Conversation

rong4188
Copy link
Contributor

Cell border is not printed when a cell has rowspan which breaks page

@stale stale bot added the stale label Jul 5, 2019
@stale
Copy link

stale bot commented Jul 5, 2019

This issue has been automatically closed since the issues in this project are mainly used for bugs and feature requests. Questions are directed to stackoverflow.

@stale stale bot closed this Jul 5, 2019
@simonbengtsson simonbengtsson reopened this Jul 7, 2019
@stale stale bot removed the stale label Jul 7, 2019
@simonbengtsson simonbengtsson self-assigned this Jul 7, 2019
@simonbengtsson
Copy link
Owner

It seems this does not actually resolve #480. The issue that can currently be seen in the rowspan and colspan example is still there when running with this fix applied. Can you send me an example of an issue that gets resolved with this fix?

@rong4188
Copy link
Contributor Author

Hi @simonbengtsson ,
I just made a fiddle https://jsfiddle.net/tjm1apc9/ which used the master branch of my fork.

Cheers,
Roni

@rong4188
Copy link
Contributor Author

@simonbengtsson
and this is the rowspan and colspan example with the fix applied.
https://jsfiddle.net/p7x2nf50/

@simonbengtsson
Copy link
Owner

Awesome! Seems like the issue you were experienced is fixed! Still looks like the issue in the colspan rowspan example is still there though?

Screen Shot 2019-07-11 at 09 33 02

@simonbengtsson
Copy link
Owner

I'll review the code a bit and then likely merge it. Can you resolve the conflicts? You can simply remove the package-lock.json and the dist files from the pull request.

@npgeomap
Copy link
Contributor

Just saw that you are working on this "page break colspan issue", wondering if it's possible to repeat the colspan content after a page break. Like this:
repeatRowSpanContent

@simonbengtsson
Copy link
Owner

Certainly doable @npgeomap. We should probably add it as an option though and not as default?

@rong4188
Copy link
Contributor Author

Well, I think it might cause trouble when it has multiple lines of text which also breaks through pages. I didn’t aware of any easy solution for that actually.

@simonbengtsson
Copy link
Owner

simonbengtsson commented Jul 11, 2019

Very true. Didn't think of that. I guess that could be considered edge cases though so limiting row height to one page is probably fine when that option is set.

@npgeomap
Copy link
Contributor

Good point @rong4188 therefore absolutely optional as @simonbengtsson mentioned.
Should I create a new feature request?

@simonbengtsson
Copy link
Owner

Sure! I have a feeling no one will look at it for the foreseeable future so if you want it I would take a shot at a PR as well 👍

@rong4188
Copy link
Contributor Author

I just found I stupidly edited everything in my master branch, so that it is very hard for me to remove some changed files from the pr. the conflicts are resolved anyway, so if this pr is merge-able can you please merge them all in then rebuild it to overwrite my dist files and package-lock.json file?

Thanks,
Roni

@simonbengtsson simonbengtsson merged commit 501e0e0 into simonbengtsson:master Jul 12, 2019
@simonbengtsson
Copy link
Owner

Perfect 👍

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

Successfully merging this pull request may close these issues.

None yet

3 participants