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

FEATURE: Use data source to list forms in inspector #506

Merged

Conversation

daniellienert
Copy link
Member

@daniellienert daniellienert commented May 8, 2016

Currently form identifiers need to be defined manually extending
the properties of TYPO3.Neos.NodeTypes:Form .
This change adds a data source which uses the
TYPO3\Form\Persistence\YamlPersistenceManager::listForms
to list all available forms automatically.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @daKmoR, @skurfuerst and @aertmann to be potential reviewers

@daniellienert daniellienert force-pushed the feature/use-datasource-to-list-forms branch from 3986632 to ac4fcfc Compare May 8, 2016 13:29
@aertmann aertmann changed the title FEATURE: Use datasource to list forms in inspector FEATURE: Use data source to list forms in inspector May 9, 2016
@aertmann
Copy link
Contributor

aertmann commented May 9, 2016

nice improvement 👍

@aertmann
Copy link
Contributor

aertmann commented May 9, 2016

this could potentially expose form identifiers you don't want to expose though

also it doesn't conflict with existing configuration of the old behavior right?

@skurfuerst
Copy link
Member

Very nice from my side -- looks very good! 👍

All the best, Sebastian

@daniellienert
Copy link
Member Author

@aertmann: You're right, the data source overwrites the manual configuration. This means, it would expose existing, but not yet made accessible forms to the editor. I didn't encounter any other conflicts.
So should this change somehow be marked as breaking / behavior changing?

@aertmann
Copy link
Contributor

@daniellienert: okay thanks for checking, I'd say that it's not really breaking since existing forms should still work, it's just that there could be some extra forms available. Thus I think it's fine not to mark it.

However the documentation needs to be adjusted http://neos.readthedocs.io/en/stable/HowTos/AddingSimpleContactForm.html

Also could make sense to describe that dataSourceIdentifier can be unset and the values configured manually if preferred.

Currently form identifiers need to be defined manually
extending the TYPO3.Neos.NodeTypes:Form properties.
This change adds a datasource which uses the
`TYPO3\Form\Persistence\YamlPersistenceManager::listForms`
to list all available forms automatically.
@daniellienert
Copy link
Member Author

@aertmann thanks for reminding me of the documentation. I removed the section about the node type configuration.

@dfeyer
Copy link
Contributor

dfeyer commented May 23, 2016

Nice one 👍

@aertmann aertmann merged commit 2344e77 into neos:master May 24, 2016
@kitsunet
Copy link
Member

Just had someone in chat say that this was a breaking change for them. As it touches configuration it stricly is always a breaking change... I guess we need to get stricter on this.

@daniellienert daniellienert deleted the feature/use-datasource-to-list-forms branch February 19, 2018 20:03
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

6 participants