Skip to content

[4.0] com_config options#24725

Merged
wilsonge merged 5 commits intojoomla:4.0-devfrom
brianteeman:com_config
Apr 28, 2019
Merged

[4.0] com_config options#24725
wilsonge merged 5 commits intojoomla:4.0-devfrom
brianteeman:com_config

Conversation

@brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Apr 24, 2019

This is for the options screens for all components. ie the view you get when you click on the options button and for global config
This PR should have no visible impact. It just changes from rendering the input and label as individual items to rendering it as a field which is what we do (almost) everywhere else

This is for the options screens for all components. ie the view you get when you click on the options button.
This PR should have no visible impact. It just changes from rendering the input and label as individual items to rendering it as a field which is what we do (almost) everywhere else
@brianteeman
Copy link
Contributor Author

Updating branch to trigger drone

@brianteeman
Copy link
Contributor Author

@zero-24 can you look at RIPS for me please

@brianteeman
Copy link
Contributor Author

updated to include global config

</div>
<?php echo $field->renderField(); ?>
<?php else : ?>
<?php echo $this->form->getInput('rules'); ?>
Copy link
Contributor

Choose a reason for hiding this comment

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

So it looks like we removed the controls class on the input before. Is that a issue here :/ Because if it's just hiding the label we can do that at the xml level and have no crazy exceptions 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.

Yes this pr is about hiding the label and yes it can be done in the xml but it would have to be done for every switcher field in every xml afaict

Copy link
Contributor

Choose a reason for hiding this comment

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

Our if statement here is based on whether the field is called permissions. That's much more specific than a switcher field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test it and see if it works. Always happy to accept better code.

@zero-24
Copy link
Contributor

zero-24 commented Apr 25, 2019

@zero-24 can you look at RIPS for me please

Drone restarted: http://ci.joomla.org/joomla/joomla-cms/17428

@@ -80,13 +80,10 @@
<?php else : ?>
<div class="control-group<?php echo $groupClass; ?>"<?php echo $dataShowOn; ?>>
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work on testing. We're getting nested control groups. Because we get one on this line and one when we call renderField

Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2019-04-27 at 15 51 12

Screenshot. Admittedly it renders ok. but clearly not correct

Copy link
Contributor

Choose a reason for hiding this comment

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

Done a PR to your branch for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I recover from man flu I will take a look

@wilsonge wilsonge merged commit 8c38f73 into joomla:4.0-dev Apr 28, 2019
@wilsonge
Copy link
Contributor

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone Apr 28, 2019
@brianteeman
Copy link
Contributor Author

Thanks

@brianteeman brianteeman deleted the com_config branch April 28, 2019 16:35
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.

4 participants