Repository navigation
feat(crop): 5:4 slice on the aspect dial, 4:5 on upright pictures for Instagram (#542) - #543
Conversation
…ram) KnifeLemon#542, from KnifeLemon#517: Instagram's portrait size is 4:5, which the dial didn't have. 5:4 sits between 1:1 and 4:3 and turns like the other fixed ratios, so an upright picture gets 4:5 and the hub swaps it. No new strings.
KnifeLemon
left a comment
There was a problem hiding this comment.
Thanks @cre8ive-marketing! The change is nicely contained: 5:4 goes through CropMath like the other fixed ratios, so the hub, CycleAspect, the glyph and the labels all pick it up without special cases. No new strings and no settings change. The new test checks the label, the canvas ratio and the frame on a real upright picture. Adding the 5:4 slice is fine with me.
src/Filee.App/Controls/AspectDial.cs,RingSegment.Render: a seventh slice breaks the Russian "Free" label in the middle of the word. The text width is2 * ((inner + outer) / 2 + 6) * sin(span / 2) - 12. With seven slices instead of six that drops from about 79 px to about 67 px, and a long single word only shrinks down to 9.5. Incrop-dial-ru-light.png(fromOne_picture_shows_the_frame_its_size_and_the_aspect_dial), "Произвольные" now wraps as "Произвольны" / "е"; on main it fits on one line. Please make the longest one-word labels fit again, either with a lower size floor or with more of the slice's width, and check theruscreenshot. It's also worth checking languages that aren't inLooks, such aspl("Oryginalne") andne("स्वतन्त्र").AspectDial.cs, theAspectGlyphsummary still says "six slices". It is seven now. "A slice per ratio" would stay true if more are added later.
With those two fixed it's good to go once CI is green.
Russian's "Произвольные" needs 9.4 px with seven slices, so the floor goes from 9.5 to 9. A test checks every language's free and original names against the slice width. The AspectGlyph summary no longer counts the slices.
|
@KnifeLemon, thanks for the review! Both are fixed in 53f2760:
Crop tests (51) pass and format is clean. |
Russian's label needs 8.7 px on CI's runners (9.4 on a desktop): the fonts differ, so the floor leaves room below both.
|
@KnifeLemon, the new label test caught a difference on CI. There, Russian "Произвольные" needs 8.7 px; on my desktop it needs 9.4, because the runners' fonts differ. 2d3becb lowers the floor to 8 so it fits on both, and the label stays on one line. The packaging jobs failed only because that test did. |
|
@KnifeLemon, both points of your review are fixed (8 px floor for long one-word labels, with a test over every language; "a slice per ratio"), and CI is now green on every check. Could you take another look and merge it into main if it is good? The two #542 follow-ups (ratio across a batch, custom ratio) come after it, as agreed. |
KnifeLemon
left a comment
There was a problem hiding this comment.
Thanks @cre8ive-marketing! Both points are done: "Произвольные" stays on one line, the AspectGlyph summary no longer counts slices, and Every_language_fits_its_longest_one_word_ratio_name_on_a_slice_without_breaking_it keeps future translations from breaking a word. Pulling LabelWidth out so the test and Render share it is a nice touch.
One small note, not blocking: the MinLabelSize comment says the CI runners' fonts differ, but the Windows and the Ubuntu jobs both measured exactly 8.66. The dial uses the embedded Noto Sans, so 8.66 is what the app draws everywhere. If you touch the file again, the comment could just say that Russian needs about 8.7 on seven slices.
It can be merged once CI is green.
Closes #542 (asked in #517).
@KnifeLemon, this is for your approval: it adds one slot to the crop's aspect dial. If you like it and it fits, it could go into the upcoming release. If not, I'll close it.
What it does
Instagram's portrait size is 4:5, which the dial didn't have.
Changed:
CropMath(the ratio list,CanSwap,Ratio)RingSegments inCropOverlayWindow.axamlAspectDial/CropOverlayViewModeldocs/ARCHITECTURE.mdTwo questions (not in this PR)
The_next_file_starts_free_and_full_in_view_and_canvas_aliketest says. A batch for Instagram means picking 4:5 again for every picture. Should the chosen ratio carry over to the next file?I'll do either one after your OK.
Tests
CropMathTests: 5:4 and its portrait 4:5, the swappable list, the dial order.An_upright_picture_crops_at_instagrams_4_to_5_from_the_5_to_4_slice: clicks the 5:4 slice on an upright picture and checks the label (4:5), the canvas ratio and the frame. It also saves the screenshot above.dotnet format --verify-no-changesis clean.dotnet test: Platform 79, Core 602, CLI 27, Setup 155, Engines 1821 and App 1082 pass, none fail.