Skip to content
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

22478 button presenter does not take into account its color #1820

Conversation

juliendelplanque
Copy link
Contributor

@juliendelplanque juliendelplanque commented Sep 21, 2018

Fixed the bug.

Had to introduce UIThemeDecorator which allows one to dynamically override some parts of a theme.

This is used to override buttons colors.

Now, SpecDemo red button is working:

spec ui framework demo

And with dynamic spec builder you can do funny stuff:
untitled window

@juliendelplanque
Copy link
Contributor Author

@pavel-krivanek you might be interested to review this change since you did SpecDemo :-)

Copy link
Member

@guillep guillep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the comments I did there, I like what the change allow us to do. And there are tests.

]

{ #category : #'reflective operations' }
UIThemeDecorator >> doesNotUnderstand: aMessage [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to override doesNotUnderstand:?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the goal is to let the UITheme decorated continue to answer to style messages that are not overridden by the decorator.
If you have another idea on how to do it without modifying the whole theme mechanism, I'd be happy to implement another solution.

property: #buttonNormalFillStyleFor: returnsValueOf: normalColorBlock;
property: #buttonMouseOverFillStyleFor: returnsValueOf: normalColorBlock;
property: #buttonPressedFillStyleFor: returnsValueOf: clickedColorBlock;
yourself).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite like the usage of blocks here...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at the implementation of UITheme>>#buttonNormalFillStyleFor:, I just wanted to keep a similar behaviour.

However, if it is ok to do it, UIThemeDecorator allows one to do something like:

themeDecorator property: #buttonNormalFillStyleFor: returnsValueOf: (SolidFillStyle color: Color red)

@pavel-krivanek
Copy link
Collaborator

Actually, I'm not sure if we should support Spec colors this way at all... But since this PR is limited only to the MorphicAdapter of Button presenter, I think we can integrate it as (hopefully) temporary solution

@MarcusDenker MarcusDenker merged commit 4b8eac9 into pharo-project:development Sep 26, 2018
@juliendelplanque
Copy link
Contributor Author

@pavel-krivanek what kind of support for colours do you expect in Spec?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants