-
-
Notifications
You must be signed in to change notification settings - Fork 377
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
docs(examples): simplify docs using new layout methods #731
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #731 +/- ##
=======================================
- Coverage 92.3% 92.3% -0.1%
=======================================
Files 55 55
Lines 14840 14836 -4
=======================================
- Hits 13703 13699 -4
Misses 1137 1137 ☔ View full report in Codecov by Sentry. |
Waiting on #729 to be finalized for naming. |
4d2a04a
to
40e1d40
Compare
Removed the helper methods. They don't make things particularly clearer and have problems with polluting the imports. They're tough to put in the prelude due to the common names (min/max/length), which overall suggests that we're better without them. Diff when removed was 254 lines added, 258 removed, so the benefit on LoC is negligible. |
5df3a23
to
0d1daaf
Compare
Rebased on the current version of main with Rect:split() |
0d1daaf
to
4d84bc5
Compare
Regarding testing this, the easiest way is to run |
Use the new `Layout::horizontal` and `vertical` constructors and `Rect::split_array` through all the examples.
4d84bc5
to
350cf68
Compare
Rebased now that #729 is merged. |
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.
Looks good, I can confirm all examples are working and layouts are visually ok.
Co-authored-by: Valentin271 <[email protected]>
I think it is a little confusing that we have
and we also have
Is there a reason we couldn't always use the second form? |
The area.split(layout) form requires knowing the number of rects that will be returned beforehand. The layout.split(area) form does not. The places where I've left that are places where there are a dynamic number of areas in which to split to. I also left a couple of more complex examples alone (IIRC it was in the old demo). You're right - this could do with some docs in |
Use the new
Layout::horizontal
andvertical
constructors andRect::split_array
through all the examples.