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

NEW ListboxField react field schema #10842

Conversation

andrewandante
Copy link
Contributor

@andrewandante andrewandante commented Jun 28, 2023

Adds the required methods for ListboxField to have a React Component (in this case, for asset-admin)

Companion PR to silverstripe/silverstripe-admin#1533

Blocker for #10819

Issue

@GuySartorelli
Copy link
Member

Unit test failures seem relevant

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

Mostly looks good to me and works locally. There's some CI things to clean up, and a couple of small things in the code.

src/Forms/ListboxField.php Outdated Show resolved Hide resolved
src/Forms/ListboxField.php Outdated Show resolved Hide resolved
src/Forms/ListboxField.php Outdated Show resolved Hide resolved
src/Forms/ListboxField.php Outdated Show resolved Hide resolved
@andrewandante andrewandante force-pushed the ENH_add_listboxfield_react_component_schema branch 3 times, most recently from 7f32db5 to 4037f21 Compare July 3, 2023 23:02
@andrewandante andrewandante force-pushed the ENH_add_listboxfield_react_component_schema branch from 4037f21 to e1d10a0 Compare July 3, 2023 23:28
@andrewandante
Copy link
Contributor Author

Think we are there now:

listbox_react

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

Looks good, works well locally. Thank you for this!

@GuySartorelli GuySartorelli merged commit 66f2df2 into silverstripe:5 Jul 4, 2023
11 checks passed
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.

3 participants