refactored to adapt to ofxGui changes #21

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
@arturoc

arturoc commented Aug 3, 2015

This adapts ofxGuiExtended to the changes in openframeworks/openFrameworks#4150

I've added to ofxBaseGui 2 new properties, a layout (which right now can be vertical and horizontal) and a boolean inContainer. that solves all the cases where parent was being used.

then i've refactored you controls a bit, mostly to get rid of the places where it used parent, and added a config struct to some of the controls. i've also changed the setup methods to return a reference instead of a pointer and even removed some setup methods in favor of the new templated add methods in ofxGuiGroup.

the exampleControls is also refactored to the new syntax.

this is not complete but just as a first step towards integrating it with ofxGui. don't merge it into your master cause that will break the addon with current OF but just to see what you think about the changes.

i think we could:

  • integrate ofxPanelExtended and ofxGuiGroupExtended's functionality in ofxPanel and ofxGuiGroup
  • adding an ofxGuiSpacer is a little verbose with the new syntax so it would be good to also integrate it into ofxGuiGroup so we can do: panel.addSpacer(40) for example.
  • i'm not very sure about the utility of a rotary slider, the canvas and the master slave control but the rest of them seem really useful so i think we could merge then with ofxGui right away or at least with my branch and then into master once 0.9 is out
@frauzufall

This comment has been minimized.

Show comment
Hide comment
@frauzufall

frauzufall Aug 3, 2015

Owner

Ahh I just did the same thing , added this changed to the new layout in a new branch of your openframeworks refactor-gui branch.. https://github.com/frauzufall/openFrameworks/tree/refactor-gui-extended

It took a while and I wanted to have separate commits for all changes I made to ofxGui to make it easier to discuss... Wanted to write about that this evening ;)

Owner

frauzufall commented Aug 3, 2015

Ahh I just did the same thing , added this changed to the new layout in a new branch of your openframeworks refactor-gui branch.. https://github.com/frauzufall/openFrameworks/tree/refactor-gui-extended

It took a while and I wanted to have separate commits for all changes I made to ofxGui to make it easier to discuss... Wanted to write about that this evening ;)

@frauzufall

This comment has been minimized.

Show comment
Hide comment
@frauzufall

frauzufall Aug 3, 2015

Owner

Sorry we should have communicated that somehow to avoid doing everything twice.. But on the other hand I did what you said, integrating ofxGuiGroupExtended and ofxPanelExtended into ofxGuiGroup and ofxPanel and since you also thought about all the changes we can discuss the differences between our versions and have a reasoned result.

For example I thought about the ofxToggleGroup for exclusive toggles, but the problem is that only this group and derivatives can use this functionality. ofxGuiMatrix can also be filled with different controls than toggles, but it should have the exclusive toggles option. That's why I put it in ofxGuiGroup. The drawback is that ofxGuiGroup gets pretty big. That's why I wanted to put this functionality to ofxToggle, but this is not possible without having a parent object.

But I think we should discuss everything in https://github.com/frauzufall/openFrameworks/tree/refactor-gui-extended because it is the merged version of the two addons, what do you say? This addon should keep the old version as long as the new ofxGui is not merged with the OF master.

Owner

frauzufall commented Aug 3, 2015

Sorry we should have communicated that somehow to avoid doing everything twice.. But on the other hand I did what you said, integrating ofxGuiGroupExtended and ofxPanelExtended into ofxGuiGroup and ofxPanel and since you also thought about all the changes we can discuss the differences between our versions and have a reasoned result.

For example I thought about the ofxToggleGroup for exclusive toggles, but the problem is that only this group and derivatives can use this functionality. ofxGuiMatrix can also be filled with different controls than toggles, but it should have the exclusive toggles option. That's why I put it in ofxGuiGroup. The drawback is that ofxGuiGroup gets pretty big. That's why I wanted to put this functionality to ofxToggle, but this is not possible without having a parent object.

But I think we should discuss everything in https://github.com/frauzufall/openFrameworks/tree/refactor-gui-extended because it is the merged version of the two addons, what do you say? This addon should keep the old version as long as the new ofxGui is not merged with the OF master.

@arturoc

This comment has been minimized.

Show comment
Hide comment
@arturoc

arturoc Aug 3, 2015

:) great no worries, i also wanted to give it a try to see if the changes i did in ofxGui some days ago made sense. i'll take a look at your changes but if you've done more or less the same i think we are good. i'll close this then

arturoc commented Aug 3, 2015

:) great no worries, i also wanted to give it a try to see if the changes i did in ofxGui some days ago made sense. i'll take a look at your changes but if you've done more or less the same i think we are good. i'll close this then

@arturoc arturoc closed this Aug 3, 2015

@@ -378,8 +360,8 @@ void ofxGuiGroupExtended::allowMultipleActiveToggles(bool allow) {
}
bool ofxGuiGroupExtended::setActiveToggle(ofxToggle* toggle) {
- if(!toggle->getParameter().cast<bool>().get()) {
- (ofxToggle)*toggle = true;
+ if(!(*toggle)) {

This comment has been minimized.

@frauzufall

frauzufall Aug 3, 2015

Owner

haha I sometimes tend to do things more complicated than necessary..

@frauzufall

frauzufall Aug 3, 2015

Owner

haha I sometimes tend to do things more complicated than necessary..

- int decimalPlace=3;
- ofParameter<string> label;
+ int decimalPlace;
+ ofParameter<float> value;

This comment has been minimized.

@frauzufall

frauzufall Aug 3, 2015

Owner

you are right, this parameter and label mixup should be changed in my version as well

@frauzufall

frauzufall Aug 3, 2015

Owner

you are right, this parameter and label mixup should be changed in my version as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment