Skip to content
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

FIX TextCheckboxGroupField should passthrough if it has only one child #515

Conversation

ScopeyNZ
Copy link
Contributor

Currently if a developer removes the checkbox it will render a small gap on the side of the text field. This will prevent that.

cc @scott1702

@ScopeyNZ ScopeyNZ force-pushed the pulls/4.0/field-is-an-only-child branch 2 times, most recently from 0162cfa to 91e70a6 Compare November 20, 2018 03:36
@ScopeyNZ
Copy link
Contributor Author

First try!

@scott1702
Copy link
Contributor

Can confirm this works for me! Should be worth noting that this fix is only required because the only way we've been able to remove the isDisplayed checkbox is with:

$fields->removeByName('ShowTitle');
$fields->fieldByName('Root.Main.Title')->setTitle('Title');

@ScopeyNZ
Copy link
Contributor Author

Tests should be fixed by #516

@robbieaverill
Copy link
Contributor

So it becomes a text checkbox group without a checkbox? Wouldn’t it be better to remove the group and use a simple text field for the title?

@ScopeyNZ
Copy link
Contributor Author

Yeah I was noting the redundancy of this fix as I did it. Currently it's not so easy for reasons I haven't quite nailed down yet. I figure this at least handles this situation in case somebody does similar...

@robbieaverill
Copy link
Contributor

Probably because CompositeFields don't have names, so they're hard to target via $fields->removeByName() 😆

Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

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

Could use a comment, but not a blocker I did this while rebasing to get the tests green.

@robbieaverill robbieaverill merged commit b8698c6 into silverstripe:4.0 Nov 20, 2018
@robbieaverill robbieaverill deleted the pulls/4.0/field-is-an-only-child branch November 20, 2018 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants