-
Notifications
You must be signed in to change notification settings - Fork 685
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
Fixes #3469, #3473, #3482 - Dim.Auto
fixes and Pos
/Dim
refactor to support TGD
#3480
Conversation
…s Frame.Size == ContentSize
Disabled diagnostic spew
Upgrade AllViewsTester
Upgrade AllViewsTester
Dim.Auto
gets confused when one of Height/Width
is AbsoluteDim.Auto
gets confused when one of Height/Width
is Absolute
Dim.Auto
gets confused when one of Height/Width
is AbsoluteDim.Auto
fixes and Pos
/Dim
refactor to support TGD
Pos.At -> Pos.Absoulte for consistency
I don't think one can spend enough time on XML docs. Nothing drives clarity of thought more than writing clear, crisp, and complete English sentences.
|
@dodexahedron I believe I've addressed all your feedback with the exception of those where I've left the comment unresolved with a question I added. Really good stuff, and I appreciate your diligence. |
Man I swear I had responded to this, but I must have mis-clicked. I wrote it on another PC, though, so I'll look when I'm able to on that. The gist was "Agreed - XmlDoc is good and not optional in the finished product, but changes are likely to result in lost/wasted effort." If you don't mind, no biggie. Just trying to save you some work and time. :) |
Specifically for this PR, I mean. Not as a SOP. |
Now that I've (I think) acked existing comments, I'm diving back in to pull and look at the current code. :) My intent for this pass is, unless there are any biggies that should block the refactor til being fixed, to comment on small things but otherwise target a green light for this PR so we can work on things that are actual changes outside just the refactor (several of which we already let sneak in, but oh well). |
After I cram dinner down my neck, that is. 😆 |
Not done yet. I fell asleep at some point, which was much needed (what a week, man). And it's 3AM, so I'm headed back to bed. Should have stuff for ya tomorrow (today). Good work. 🙂 |
😭 I lost a review I was working on. I was on my laptop, booted into Ubuntu, writing in Kate, just in case github or my browser acted up.1 Then the machine hard locked up for a hardware reason I now need to look up the light flash codes for in the repair manual and probably get Dell on-site to replace if necessary. 😤 At least there wasn't anything profound in it, so I can recreate it on another machine. I'll probably do that til I fall asleep, so half an hour or so tonight. Footnotes
|
Ok maybe not so much tonight. After going through some of my github inbox it's already just about midnight, so I'll pick it up tomorrow. Was planning on Tuesday having dedicated time for TG anyway. 🙂 |
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.
Just a few, this time. After sleeping on it, I scaled it further back from what I had before my laptop exploded, because of scope creep.
After these tweaks, I'd say it's fine to merge.
Then we can get to other changes & enhancements.
I'll start filing new issues targeting this after it is merged. |
Added [GenerateEnumExtensionMethods] to DimAutoStyle. Reordered DimAutoStyle.
🥳 Once I finish migrating the Roslyn stuff, I'll get to that draft issue I started a couple days ago. |
Fixes
Dim.Auto (DimAutoStyle.Text)
confusesTextFormatter
whenWidth
is absoulte andHeight
is auto #3469Dim.Auto
screws upPos.AnchorEnd()
#3473Dim.Auto
forContent
including subviews. #3451Proposed Changes/Todos
Dim.Auto
screws upPos.AnchorEnd()
#3473 with unit tests.The problem was I had removed some code I thought was dead in
LayoutSubViews
that forced a double calc of pos/dim in order to makeDimAuto
work when subviews depended on the view's dims. That code was very much "prototype" and didnt belong there anyway. I now have a much more elegant soution baked intoSetRelativeLayout
where if the Dim is DimAuto, we calcuate the Dim first (with Pos == 0) and then the Pos (using the resulalant dim).This all highlighted clumisness in having
ContentSize
be nullable. So I changed it to non-nullable and added aSetContentsSize()
method. The API is still a bit awkward to use so I may refactor it again at some point.Refactor
AutoSizeTrue/False
unit tests into new tests that make sense.New issue: Dim.Auto
min:
defines min size of theFrame
. This should be min size ofContent.Size
!I'm pretty confident #3473 is now fixed here.
Instead of focusing on #3469 (which is partially fixed), I'm now going to use this PR to also address:
v2 Pos classes need to support interrogation for TGD #3482 in order to simplify @tznind's work on TGD.
Remove nested class heirarchy in PosDim.
Make the Pos/Dim types public - with API documentation that warns of pitfals of using them in normal situations... should not be needed!
Make settings on the Pos/Dim types be publicly available (read only).
Verify
Dim.Auto (DimAutoStyle.Text)
confusesTextFormatter
whenWidth
is absoulte andHeight
is auto #3469 is fixed.Pull Request checklist:
CTRL-K-D
to automatically reformat your files before committing.dotnet test
before commit///
style comments)