Princeton-LSI-ResearchComputing / tracebase

Mouse Metabolite Tracing Data Repository for the Rabinowitz Lab
MIT License
4 stars 1 forks source link

Added code to autofit comment boxes to the text content. #938

Closed hepcat72 closed 5 months ago

hepcat72 commented 5 months ago

Summary Change Description

This was a quick little tweak to the comment box dimensions.

I basically used textwrap.wrap(), combined with a fixed width font to be able to predictably calculate box dimensions. It's not perfect, but any ill-sized comment windows can be manually tweaked by users if necessary.

Affected Issues/Pull Requests

Review Notes

See comments in-line.

Checklist

This pull request will be merged once the following requirements are met. The author and/or reviewers should uncheck any unmet requirements:

hepcat72 commented 5 months ago

Neat! Seems to wrap the lines effectively and predictably. On my system, it still looks good, but, the font size calculations don't appear to hold up in all cases. I'm using LibreOffice, so that might be part of the issue, and I'm using Linux, which uses open-source fonts. See example: image

Yeah, I figured there may be some variability, so I erred on the side of a little extra. The wrapping is based on the number of characters and the box size was something that I tweaked by trial and error. I was surprised there wasn't an auto fit option like you can do for the cells.

lparsons commented 5 months ago

Yeah, I figured there may be some variability, so I erred on the side of a little extra. The wrapping is based on the number of characters and the box size was something that I tweaked by trial and error. I was surprised there wasn't an auto fit option like you can do for the cells.

I also somewhat found the original somewhat nicer to look at just because the font was a bit nicer, Courier isn't the prettiest. But either way is OK with me.

hepcat72 commented 5 months ago

Yeah, I figured there may be some variability, so I erred on the side of a little extra. The wrapping is based on the number of characters and the box size was something that I tweaked by trial and error. I was surprised there wasn't an auto fit option like you can do for the cells.

I also somewhat found the original somewhat nicer to look at just because the font was a bit nicer, Courier isn't the prettiest. But either way is OK with me.

I also preferred the original font, however the wrapping is based on the number of characters. With a variable width font, the line breaks would have looked off. Maybe I'll adjust the background color though.

hepcat72 commented 5 months ago

Oh, I meant to say that I could use a different fixed-width font if you have a suggestion. I just know that courier is pretty universal.

lparsons commented 5 months ago

Oh, I meant to say that I could use a different fixed-width font if you have a suggestion. I just know that courier is pretty universal.

Not really, I think we'd run into people missing the font if we tried others. Will the text wrap itself if you set a width on the comment box? That would be really nice, but all this is just gravy atm.

hepcat72 commented 5 months ago

Oh, I meant to say that I could use a different fixed-width font if you have a suggestion. I just know that courier is pretty universal.

Not really, I think we'd run into people missing the font if we tried others. Will the text wrap itself if you set a width on the comment box? That would be really nice, but all this is just gravy atm.

The text will soft-wrap if you manually narrowed the width, but there are hard-returns that are added to the text of the comment, which would make it look ugly if soft-wrapping started to occur.

The reason for the hard-wrap strategy was that I needed dimensions to be able to calculate a box height. I couldn't predict how many lines there would be using soft-wrap and a variable width font, so I used the textwrap core package in python to wrap based on the number of characters and kept track of how many lines were output and what the number of characters was on the longest line. Then, using a fixed witdh font, the dimensions were calculable.

hepcat72 commented 5 months ago

I suppose I could set a width and use the hard-wrap strategy to estimate the height, but use the un-hard-wrapped text so that resizing would look OK. Maybe in another PR in the future...

lparsons commented 5 months ago

Oh, too bad you can't just set a width and let the height automatically adjust. That would have been ideal, but this is fine. We can fine tune as we go.

hepcat72 commented 5 months ago

Oh, too bad you can't just set a width and let the height automatically adjust. That would have been ideal, but this is fine. We can fine tune as we go.

Exactly. That's what I meant with "I was surprised there wasn't an auto fit option like you can do for the cells.". I looked. Pretty exhaustively. I even looked through the code in the xlsxwriter package.

hepcat72 commented 5 months ago

You can't even automatically adjust the comment box height in excel itself. It only have static settings. Nothing dynamic.