-
Notifications
You must be signed in to change notification settings - Fork 6
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
Toplo with themes and skins #156
Conversation
Thanks @Nyan11
Ok, I have a review with comments.
No problem this is an alpha version and this is normal to introduce problems between them. |
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.
Comments + waiting for pharo-graphics/Toplo#97 + waiting for alpha 4
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.
Ok for this new baseline.
I don't think this is usefull to have separate version because users have to use Toplo... however if we succeed to execute CI in these two baselines it is a good practice to have a better architecture.
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.
Haha, do you have the problem on your PC ?
pharo-graphics/Bloc#356
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.
Need to update CI scripts.
self toploPackages: spec. | ||
|
||
"Groups" | ||
spec group: 'default' with: #( 'PyramidBloc' 'PyramidToplo' ). "complete version" |
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.
Rename PyramidBloc->Bloc and PyramidToplo-> Toplo
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.
Oups seems not possible to do that :(
TODO: This script should load with Toplo by default: Metacello new
baseline: 'Pyramid';
repository: 'github://OpenSmock/Pyramid:main/src';
load |
@Nyan11 I think the example PyramidToploExamples>>buttons need to be recreate because there is an exception on opening. And this is strange because ToElement have serialized children :/ |
I will check it |
We need to load the dev branch of Toplo but currently the dev branch of Toplo is bugged |
@labordep Everything should work, but you need to merge first: OpenSmock/Toplo-Serialization#1 and: pharo-graphics/Toplo#123 . You should check on your side if everithing is ok before merging. Toplo-Serialization -> If we have a ToElement, it should serialized itt's children. If we have anything with TToPlaceHolder trait it should serialize it's children. Everithing else should not serialize it's children. Toplo -> There is a bug that make impossible to instanciate ToElement currently on master ... (it also break the tests of Toplo-Serialization) |
Ok I can launch the example, but buttons are all the same: @Nyan11 I'm trying to reload a new image from scratch. |
Yes i did not implement a way to save the default theme for the space. |
It is complicated, since it only saves the BlElements inside the space and not directly the space ... |
Ok thanks @Nyan11, we will discuss on that later. |
Fix: #155
It is a PR to make Pyramid compatible with Toplo.
You need to validate the pull request on Bloc-Serialization at: OpenSmock/Bloc-Serialization#14.
Attention when this version will be merged it will possibily break all projects that currently use Pyramid.
It change the serialization (all examples are brokens + all specials widget (switch and visibility).
It change deep mechanics inside Pyramid.