-
Notifications
You must be signed in to change notification settings - Fork 693
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 #2114. Adds View.ShadowStyle
- Enabled but not turned on by default
#3376
Conversation
@dodexahedron Can you please look at this PR and help me out? I mentioned a similar issue the other day. This PR was originally rebased with #3415. #3415 was just merged into v2_develop. I then rebased this PR on to v2_develop
However, if you look at the "Files Changed" in this PR on Github it is comparing to an old v2_develop. For example, it things Aligner.cs is a new file: I am sure I'm doing something wrong with how I'm rebasing. Do you see anything? |
@dodexahedron did you see my question to you above? |
This illustrates an issue with my current design here: Need to figure out how to have the shadow account for whatever was drawn below. Right now, it blindly overwrites and thus doesn't look right in some situations. I think the right design is to have Margin.Draw get called by Application, not the View's SuperView. |
That's kinda been a long-standing conundrum, hasn't it? What order to draw things in, I mean. Since we don't have an explicit concept of Z index, it's all just order dependent, which is something that can change at runtime as views are created and destroyed. As far as what color to use, I've got blend code that's aimed at true color but works for lame color too. I may have even put at least some of it in when I did that work on the Color type a while back. 🤔 |
I have these effects turned off by default and plan on merging it that way... Why? Because we have so many fragile unit tests that can't deal with a) Button not being 1 row high. Same thing goes for Please, please, please, lets not write any more unit tests that depend on visual styling and if you're in a unit test as part of a PR that does this re-write it! |
View.ShadowStyle
- Enabled but not turned on by default
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.
Excellent work.
…d_project_QoL_tweaks_1 Fixes gui-cs#3557: Mostly non-impacting minor changes to the csproj, a fix for nuget.config, and some design-time QoL stuff
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.
A tip for test cases you need to skip.
// " C 0 A " | ||
[InlineData (-1, 0, 0)] | ||
[InlineData (0, 1, 1)] | ||
[InlineData (1, 0, 1)] // BUGBUG: This should be 1,1,1. We need to fix the logic in the Shortcut class. |
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.
As mentioned here (re-posting here in case the commit comment is not easily visible):
When you want to skip a test temporarily, feed a string to the Skip property, in the attribute constructor, as shown below and in the linked commit comment.
It's something attribute constructors can do that normal ones can't, and is basically like using an initializer.
For this one, this code, instead of the line with the comment and altered data to make it pass, will instead skip and print a message, without failing and without having to feed it a fake value that may or may not continue to pass if other things change:
[InlineData (1, 1, 1, Skip = "// BUGBUG: We need to fix the logic in the Shortcut class.")]
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.
And it doesn't lie about the test results, either. 😅
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.
I didn't know that was there til just when I looked at this.
But it occurred to me that NUnit has that capability, and it isn't exactly an uncommon thing to need to do, so surely xUnit must have it?
And sure enough, it does. :)
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.
Actually, you know what?
I'll make a tag for the todo list specifically for skipped tests that looks for something like
\[\((?:.*Skip *\= *\")(.*)\" *\)\]
(or whatever I have to tweak that to, to make it just match the string in the actual regex).
That'll give them their own category so they don't add noise to the others.
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.
Booooooo. The ToDo list feature does not support matching in generic code elements, so even though this pattern quite nicely matches the string after a Skip = "
, it doesn't work to tag it because it isn't a comment, identifier, or string.
That said, I can just make it look for a specific string in them, like "Skipping".
Oh well.
I'll just let strings match then. Same tags available.
Fixes
Button
and other views #2144Todo
Margin
to have a "Shadow" Style - When enabled draws the correct half-height block glyphs in the right placesButton
to useMargin.EnableShadow
Button
with shadow work reliably and automatically.Margin.Draw
get called byApplication
, not the View's SuperView.EnableShadow
on other Views where it may make sense visually.Background
When I created Frames (now Adornments) i had a vision that
Margin
could be used to enable the 3D effect that we had mostly working in v1, but got negated with all the changes to v2.I had a shower thought recently on this and decided to quickly code a prototype up to see if my instincts were right.
This PR demonstrates that I was. The concept is leveraging Margin like this:
Pull Request checklist:
CTRL-K-D
to automatically reformat your files before committing.dotnet test
before commit///
style comments)