Skip to content

Improvement#79

Merged
brianteeman merged 2 commits intobrianteeman:com_configfrom
wilsonge:config_fields
Apr 28, 2019
Merged

Improvement#79
brianteeman merged 2 commits intobrianteeman:com_configfrom
wilsonge:config_fields

Conversation

@wilsonge
Copy link
Copy Markdown

@wilsonge wilsonge commented Apr 27, 2019

Removes code and prevents the double control-group stuff.

However comes with different issues on the permissions fields:

  1. Your PR is missing if there's more than 1 field in the permissions fieldset.
  2. Also in your current PR (but more unlikely practical use case) we've removed the ability to have a showon based on the permissions field (it technically existed before)

My PR has the issue that it fixes that but effectively reintroduces the controls class which means the permissions field is shifted left. So I gave it it's own class revert-permissions and overriding the margin-left. Not convinced it's amazing but it works :/

@brianteeman
Copy link
Copy Markdown
Owner

Not convinced it's amazing but it works :/

That was my view of my own code

Your PR is missing if there's more than 1 field in the permissions fieldset.

Can you give me an example of where we have that

@wilsonge
Copy link
Copy Markdown
Author

Nothing I'm aware of in core. It seems relatively unlikely because we'd always be hiding the label. But before we did handle it and now we don't

@brianteeman brianteeman merged commit 1eaae4b into brianteeman:com_config Apr 28, 2019
@brianteeman
Copy link
Copy Markdown
Owner

Thanks @wilsonge

@wilsonge wilsonge deleted the config_fields branch April 28, 2019 15:56
brianteeman pushed a commit that referenced this pull request May 26, 2020
* Add codeowners

* Rename transition tab options to actions
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.

2 participants