-
Notifications
You must be signed in to change notification settings - Fork 101
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
Format rotated cell text #32
base: master
Are you sure you want to change the base?
Conversation
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 That's exactly what I was expecting. Flipping the behavior. |
👍 +1 Impressive work!! |
@@ -1,2 +1,3 @@ | |||
Gemfile.lock | |||
manual.pdf | |||
rotate.pdf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like that line wasn't meant to be committed.
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. 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: 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. |
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. |
@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. |
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:
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. |
@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: 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 One thing that would help a lot is a high-level overview of what's going on in the As a general note I'm a little nervous that it's a subclass of |
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: 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! |
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} It looks like cell width is still calculated by regular text width and not by rotated width. So I assumed I could just set 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} |
I'm seeing the same behavior as @hmt |
I just popped into this issue as well, @hmt, @JoshTGreenwood did any of you managed to get past the width problem? |
I came here following this bug report prawnpdf/prawn#409. I am using |
@marcusramberg @hmt @JoshTGreenwood I've found that setting |
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:
What remains: