Repository navigation
FIX: layout for mixed descent multiline text objects - #11499
Conversation
8df6787 to
6b75057
Compare
2f32a88 to
2b30036
Compare
2b30036 to
e7420c6
Compare
|
Squashed, and added boxes around the text in the mulitline test per @efiring suggestion... |
efiring
left a comment
There was a problem hiding this comment.
This is making important fixes so it should get in ASAP. I have only some trivial cleanup suggestions, mainly pointing to a few comments that seem both verbose and unclear.
| renderer = fig.canvas.get_renderer() | ||
|
|
||
| def draw_box(ax, tt): | ||
| if 1: |
There was a problem hiding this comment.
Looks like this conditional is left over from development.
| ymin = horizLayout[-1][1] | ||
| ymax = horizLayout[0][1] + horizLayout[0][3] | ||
| # Bounding box definition: | ||
| # by defeinition, ymax is 0 |
There was a problem hiding this comment.
Typo in comment. Also, a single line of comment should be enough here.
| # by defeinition, ymax is 0 | ||
| ymax = 0 | ||
| # ymin is the baseline of the last line subtract the descent of the | ||
| # last line. |
There was a problem hiding this comment.
I don't understand the comment; can you condense to one line, and clarify?
There was a problem hiding this comment.
Reworded, though I admit that I don't completely remember what this meant. This code is very convoluted, and could maybe do with a refactor now that we know what its supposed to do. ...
TST: add another multiline test DOC: fix comments
e7420c6 to
e3fd995
Compare
|
Pinging for a second review on this... |
PR Summary
Closes #11498 partially closes #11468
WIP: this'll fail some tests that depended on the previous bad behaviour...EDIT This indeed failed some tests, but I'd pretty strongly argue the tests were broken and this PR makes them correct...
Test:
Before:
With PR:
PR Checklist