Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

Make WithStyle behave as a stack #78

Merged
merged 4 commits into from
Dec 14, 2017

Conversation

cceckman
Copy link
Contributor

The current behavior is that WIthStyle sets a Style on the Painter; but when the func supplied to WithStyle is done, it resets to the default style of the entire program.

This is, IMO, the wrong behavior- and a cause behind at least why my attempt at resolving #60 wasn't working.

This uses the call stack of drawing to push/pop styles when WithStyle is called. Within the scope of a WithStyle, the style is applied; outside, it's whatever style was previously part of the Painter. It adds a demo (in examples/) of the issue, new functionality to testSurface to allow testing of colors, and a test for the expected behavior. This also removes ResetStyle, since that would essentially break the notion of style inheritance.

Demonstrates how the first 'okay' label renders with the default -
rather than the "normal" style, and the subsequent ResetStyle
reconfigures to "normal".
This allows testing of color behaviors to testSurface, and therefore
of Style application more generally.
This adds a test for the improper behavior shown in 7e7a180; WithStyle
doesn't provide a stack of styles, it just resets the style (ResetStyle)
after WithStyle is complete.
Without this commit, WithStyle makes a Painter reset to the default
rather than going back to the style it had before. This means if a
a Widget within a styled Box is also styled, the Widgets after it in the
Box would not inherit the Box's style.

Previous commits demonstrate this bug; this gets the tests to pass.
This was referenced Dec 14, 2017
@marcusolsson
Copy link
Owner

Looks good to me! Good idea with the FgColors and BgColors methods to test colors. Merging this one first, and will merge #79 once the conflicts have been fixed. Thanks for this (I take it your request went through?)!

One interesting thing I noticed though is that in the color example, the Everything is fine label has an incorrect style until the arrow keys are pressed. I'm seeing this on the master branch as well so no problem merging this PR.

@marcusolsson marcusolsson merged commit cee4f40 into marcusolsson:master Dec 14, 2017
cceckman added a commit to cceckman/tui-go that referenced this pull request Dec 14, 2017
Missed some of these when merging in marcusolsson#78.
@cceckman
Copy link
Contributor Author

Yeah, I noticed that issue as well- I haven't stepped through to figure out what's up with it. Uninformed speculation: re-using a Painter object across Draw invocations, so the Style set at the start of the redraw is the last style used rather than the default style? Or maybe I have the issue backwards.

Yes, request went through after some poking; you may see some more PRs coming up, as I'll be on vacation for a couple weeks -> time for personal projects :-)

@cceckman cceckman deleted the paintstack branch December 14, 2017 16:45
marcusolsson pushed a commit that referenced this pull request Dec 20, 2017
Missed some of these when merging in #78.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants