themovation / th-widget-pack

Theme Widget Pack
19 stars 6 forks source link

Added Themovation_Widget_Base Class #70

Closed adiraoco closed 7 years ago

adiraoco commented 7 years ago

Using child widgets seem to be quite buggy, so I felt this approach would be better.

I created a new class Themovation_Widget_Base that extends SiteOrigin_Widget. This class contains methods containing forms for the icon, link and button. Then widgets that use these fields will extend Themovation_Widget_Base.

For now I have updated the forms for Icon, Link, Button, Pricing and Services widgets. Another point I noticed is that radio fields do not do work properly in repeater fields as mentioned in #67 . So I changed this to select fields. Seems to work fine.

@ryanlabelle and @teaganm please have a look, test and let me know if you would prefer this. If you do, I can implement it fully in all widgets. Thanks

ryanlabelle commented 7 years ago

Thanks Adi, I have a few questions:

Button 1 + Button 2 + vs Buttons +

Button 1 + Button 2 +

  • What does that mean for the code that renders the button, link, icons? Will this code still be in a template or will it get moved to a php class. We would like the theme users to be able to edit markup easily if they need to. Is this possible?
  • Are we just looking at the field changes? The output is not working for me when I test., No icons or buttons showing when I test service block and pricing on that new branch.

Thanks :)

ryanlabelle commented 7 years ago

As for https://github.com/ryanlabelle/themovation-so-widgets-bundle/issues/67 the select lists work better, saving their state now vs radio.

adiraoco commented 7 years ago

Answering your questions in order -

We can have the buttons without a parent box. I'll make the change.

The code will still be in output in the same fashion as it is now. I'll be using the Themovation_Widget_Base class only to output repeating sections of forms; such as icons, links and buttons.

For now I have not linked the forms to the templates for this PR. My thinking being that I would implement it only if you like the way the form functions with this setup. I'll link the templates tomorrow for testing. :)

ryanlabelle commented 7 years ago

Thanks Adi, that looks great. One question, if the theme buyer wanted to customize a template, is there some sort of child template process? Can they make changes without editing core templates?

adiraoco commented 7 years ago

There should be some kind of filter the widget output goes through.. I'll check up and get back to you..

adiraoco commented 7 years ago

Please check the Service and Pricing widgets. I have modified the templates.

ryanlabelle commented 7 years ago

Yup, looks great!

teaganm commented 7 years ago

Looks great Adi, let's go ahead with this :)

As I was testing I just noticed a bug in the Service Block widget output:

The element "th-sb-single" should change to be one of these three:

Right now it's showing up for all of them as "th-sb-single-g"

adiraoco commented 7 years ago

@teaganm modified all widgets. I tested them and everything looks good. Please test the changes from the master branch. Thanks