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

Fresh work on nowrap cells #149

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

andyjeffries
Copy link

Based almost entirely on:

#50

Implements the wrapping, adds no more failures (than the current master fails on 3.0.4 anyway), but adds new tests and documentation.

Based almost entirely on:

prawnpdf#50

Implements the wrapping, adds no more failures (than the current master fails on 3.0.4 anyway), but adds new tests and documentation.
@andyjeffries
Copy link
Author

I took the work on #50, made it not fail any more tests than before, added new tests and docs. All seems "fine", except there's an additional border now, that I don't know how to get rid of!

Any ideas @gettalong ? Thanks.

Screenshot 2022-09-09 at 13 26 14

@andyjeffries
Copy link
Author

I've just done another example in there to debug, and if I force the left cell to have a newline, the right cell is effectively only half-height, but doesn't get the weird border mid-cell.

Screenshot 2022-09-09 at 14 54 50

@gettalong
Copy link
Member

@andyjeffries I'm not very acquainted with the prawn-table code but will have a look if I find some time.

@andyjeffries
Copy link
Author

andyjeffries commented Sep 9, 2022

Thanks anyway @gettalong , I found it though. I was calling cell in the manual which immediately draws it to the page as a standalone object (with its own height, that of the content), then passed it to the table (which drew it at the same height as its neighbouring cell).

This is now ready for review and merging (ignoring the test that failed before this one)

@andyjeffries
Copy link
Author

andyjeffries commented Sep 12, 2022

Just added whitespace as an option to nowrap. I found in my app that if I don't use nowrap it can wrap mid-word awkwardly in a table, but if i I use the new nowrap it's too long for the rest of the row to fit nicely. That is not what I intended, so now I've added another option to handle that. The manual should hopefully make it clear.

Screenshot 2022-09-12 at 11 33 24

@andyjeffries
Copy link
Author

@gettalong this is ready for review, but the request is blocked (I've been using it in production for six months now). Could you approve/review it?

@gettalong
Copy link
Member

@andyjeffries I'm sorry but I currently don't have the time for that.

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

Successfully merging this pull request may close these issues.

2 participants