prawn-table icon indicating copy to clipboard operation
prawn-table copied to clipboard

Format rotated cell text

Open straydogstudio opened this issue 10 years ago • 17 comments

I must admit, it is ludicrous how long it has taken me to get to this. But here we are.

This formats text appropriately for text rotated between 0 and 90 degrees. See below:

prawn-table

What remains:

  • Angles between 0 and -90. Should other angles be considered?
  • Right to left text. How should it behave?
  • Skewed cells (for table headers.)

straydogstudio avatar Nov 06 '14 21:11 straydogstudio

In case this is what you're asking: For right-to-left, it should look horizontally flipped, so that for example blocks 01-06 start from the top right and block 07 starts from the bottom right. If it's not, please clarify. :)

elad avatar Nov 07 '14 12:11 elad

@elad That's exactly what I was expecting. Flipping the behavior.

straydogstudio avatar Nov 07 '14 16:11 straydogstudio

:+1: +1 Impressive work!!

johnnyshields avatar Nov 08 '14 22:11 johnnyshields

The screen shots look awesome! Great work! Thank you very much for the hard work!

With new features like yours I'm trying to keep the code complexity as low as possible to ensure future bug fixes are easily possible. We had lots of issues with tables at the end of last year, and my hope is to stabilize the code base. To ensure that this is possible I need others and myself to be able to read the code as easily as possible. Would it thus be possible to refactor the code a little bit? Especially Prawn::Table::Cell::Formatted::Box? It would be awesome if you could manage to get an A from codeclimate on the added code e.g. https://codeclimate.com/github/straydogstudio/prawn-table/Prawn::Table::Cell::Formatted::Box I'd settle for a B if more refactoring doesn't benefit readability.

Also would be it be possible for you to add an entry to the manual?

Thank you once more for the awesome work that you've already done!

hbrandl avatar Nov 11 '14 14:11 hbrandl

@hbrandl: Refactored code would be nice, but formal high-level documentation of the computations we're implementing is better. Keep in mind that an "A" in code climate does not imply understandability beyond the elimination of local incidental complexity, so I expect it will only help a little bit with preventing bugs from surfacing, or re-surfacing.

practicingruby avatar Nov 11 '14 14:11 practicingruby

In other words, clean code will certainly help make fixes easier, but documentation of how things are supposed to work has both preventative effect and an educational effect that clean code can't gain us. Both are desirable, of course.

practicingruby avatar Nov 11 '14 14:11 practicingruby

@sandal I agree. Both things are important. And refactoring can also go too far. So let me relax that statement above. A is the goal. Anything less should have a reason.

hbrandl avatar Nov 11 '14 14:11 hbrandl

I really don't think this is a good idea to depend so heavily on Code Climate scores. They only measure extremely local internal quality issues!

I care much more about these kinds of things, which code climate cannot help us with:

  • How likely is it that this code is going to break existing behavior?
  • How likely is it that we're going to write new code that will depend on this feature?
  • What (mathematical) computations are we using here and why?
  • What edge cases are there to consider, and what will this code do when it encounters those edge cases (correctly render, raise an exception, incorrectly render, undefined behavior)?
  • What side effects, if any, does this code have on other systems?
  • Do our tests reflect whatever we've learned from the above questions?

Please understand that if you're emphasizing external stability, internal code quality is only an indirect measure, and it's not the first or best place to focus our energies. It can act as a good supplement to the above, but only if we give them the attention they deserve first.

practicingruby avatar Nov 11 '14 14:11 practicingruby

@sandal I've given your posting quite some thought. And I agree with you. From now on I'll ignore codeclimate metrics for prawn-table.

hbrandl avatar Nov 12 '14 10:11 hbrandl

@hbrandl: I use code climate metrics for Prawn core occasionally, but usually for my own cleanup work, rather than asking contributors to pay attention to them. I should have probably brought this up over email (and may still do that), sorry for not doing that.

So... in the interest of getting back on topic, let's take a look at the code!

@straydogstudio: I'm really excited about this feature, it's something we've been looking to add to tables for a while. But as @hbrandl mentioned, we've extracted prawn-table because it's unstable and very high complexity, so we need to be careful when merging new patches.

One thing that would help a lot is a high-level overview of what's going on in the Prawn::Table::Cell::Formatted::Box class. What I'd like to know is what documentation (if any) you used to figure out the rotation computations, and also what methods we've overridden from Text::Formatted::Box and why.

As a general note I'm a little nervous that it's a subclass of Prawn::Text::Formatted::Box, because we may accidentally break it with upstream changes (composition might be better), but we can ignore that for the moment. Right now, I just want to make sure we understand how the code works.

practicingruby avatar Nov 12 '14 12:11 practicingruby

Somehow I never got any messages that you guys responded here. I'm not very responsive! My profuse apologies. I just thought this got lost in the shuffle.

I have lots of notes on the way that it was done. And I will look everything over again. I have also spent a lot of time thinking on how it could be simplified. And spent more time thinking on right to left text. So I will document it some more.

A lot of the rotation calculations are rather tedious trigonometry work, mainly accounting for the height of the line itself and the corners of the cell. Really you are positioning a rectangle inside a rectangle.

By the way, one thing I found was that the height inside the cell essentially leaves no padding on the bottom. I did several checks and found the height was always a bit tall. Rotated text would always sit on the bottom edge. Not necessarily exactly, though. I will try to reproduce this. I did some searching but never did find where this got produced. Ostensibly the padding had been accounted for.

straydogstudio avatar Jan 31 '15 03:01 straydogstudio

@straydogstudio: Thanks. The basic idea is that if we can have some decent documentation on how the code is supposed to work, we can verify that (a) the testing is adequate and (b) that the code can be revised as needed to make it more maintainable. We'll be able to help with both of those things as long as we know how things are supposed to work.

If you can provide a code example and/or screenshot to illustrate the padding issue you saw, that would be helpful!

practicingruby avatar Feb 02 '15 13:02 practicingruby

This looks very nice but I get width problems:

data = [%w(some text that I just made up for demonstration purposes only)]                                                          
table data, :cell_style =>{:height => 80, :valign => :bottom, :rotate => 90}

will look like this: screenshot from 2015-03-07 22 46 10

It looks like cell width is still calculated by regular text width and not by rotated width. So I assumed I could just set :width => xx:

data = [%w(some text that I just made up for demonstration purposes only)]                                                          
table data, :cell_style =>{:height => 80, :valign => :bottom, :rotate => 90, :width => 30}

which then looks like this: screenshot from 2015-03-07 22 49 29

hmt avatar Mar 07 '15 21:03 hmt

I'm seeing the same behavior as @hmt

JoshTGreenwood avatar Mar 27 '15 17:03 JoshTGreenwood

I just popped into this issue as well, @hmt, @JoshTGreenwood did any of you managed to get past the width problem?

mauromorales avatar Jul 21 '15 07:07 mauromorales

I came here following this bug report https://github.com/prawnpdf/prawn/issues/409. I am using prawn-rails gem in Rails 4. Can I just pull the repo with PR and use it in Rails?

AnwarShah avatar Oct 19 '16 19:10 AnwarShah

@marcusramberg @hmt @JoshTGreenwood I've found that setting :valign creates the problem. If I remove :valign option all is fine. I also asked a question on StackOverflow before but have to answer myself http://stackoverflow.com/questions/40139848/bottom-to-top-text-in-column-of-prawn-table/

AnwarShah avatar Oct 28 '16 18:10 AnwarShah