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

Added form tabs #1887

Closed
wants to merge 8 commits into from
Closed

Added form tabs #1887

wants to merge 8 commits into from

Conversation

stollr
Copy link
Contributor

@stollr stollr commented Oct 25, 2017

Like the form groups tabs are another way to separate form fields to achieve a better overview and a cleaner interface.

Tabs can contain groups but not the other way round. The NormalizerConfigPass ensures that there are no orphan fields before the first tab definition similar to the normalizing of the groups' config and it even takes care about it if tabs and groups are used together.

In the Twig template the part, that is responsible for the display of groups, has been moved to a separate block to avoid code duplication.

Here's a screen shot showing how it looks like:
easyadmin-tabs

Tell me if there's anything bothering you so that I can fix it. I'd be happy if this PR would be merged ;-)

Like the form groups tabs are another way to separate form fields to
achieve a better overview.

Tabs can contain groups but not the other way round. The
`NormalizerConfigPass` ensures that there are no orphan fields before
the first tab definition similar to the normalizing of the groups'
config and it even takes care about it if tabs and groups are used
together.

In the Twig template the part, that is responsible for the
display of groups, has been moved to a separate block to avoid code duplication.
@laurent-bientz
Copy link
Contributor

👍 It'll be awesome, especially for translation stuff !

@grachevko
Copy link
Contributor

grachevko commented Oct 26, 2017

In my opinion this is useless in current realisation.
How validation will work? What if violations will be inside both tabs?
User will see only one, then submit form and then will see another?
How tabs well render on mobile devices?

In our project we use tabs for better navigation, but every tabs is a separated pages with it's own configuration in easy_admin.yaml.

@stollr
Copy link
Contributor Author

stollr commented Oct 26, 2017

I thought about the validation issue. One could add a special icon to a tab, which contains violations or a label to signalize violations.

@iluuu1994
Copy link
Contributor

@Naitsirch That and you should always jump to the first tab that containing a validation error. This way you'll never see a tab containing no error, looking like the submission was successful.

@stollr
Copy link
Contributor Author

stollr commented Oct 30, 2017

I have extended the tab functionality to address the notes made by @grachevko and @iluuu1994.

Here's a screenshot:
screenshot_20171030_133814

If there's a tab with form violations, this tab is activated.

Any other objections?

@stollr
Copy link
Contributor Author

stollr commented Nov 2, 2017

Sorry for adding another coding style fixing commit, but I couldn't get phpunit and php-cs working for this project. Executing vendor/bin/phpunit -c phpunit.xml.dist locally gave me

FAILURES!
Tests: 409, Assertions: 248, Errors: 196, Skipped: 7.

@javiereguiluz
Copy link
Collaborator

@Naitsirch don't worry! I'll take a look at tests when I review this proposal. Thanks!

$formTabs[$activeTab]['active'] = true;
}
};
$builder->addEventListener(FormEvents::POST_SUBMIT, $listenerClosure, -1);
Copy link
Collaborator

@yceruto yceruto Nov 2, 2017

Choose a reason for hiding this comment

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

I suggest add a listener class to make this EasyAdminFormType class more understandable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done ;-)

@@ -747,6 +747,38 @@ very advanced layouts.
This solves most of the issues, but sometimes you might be forced to also
reorder the form group positions.

Form Tabs
...........
Copy link
Contributor

Choose a reason for hiding this comment

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

too many dots

@stollr
Copy link
Contributor Author

stollr commented Dec 1, 2017

@fabpot Thanks for the catch. I've removed the superfluous trailing dots.

@victorbstan
Copy link

I would like to use form tabs, can I merge this pull request into my own EasyAdmin branch? Or will this be merged sometime soon in the main EasyAdmin repo?

@stollr
Copy link
Contributor Author

stollr commented Jan 9, 2018

@victorbstan Yes, of course you can ;-)

@javiereguiluz Would be nice if you could review this one soon.

@javiereguiluz
Copy link
Collaborator

@Naitsirch thanks a lot for contributing this feature. It was a stellar first contribution to this bundle. And to be completely honest, I would have never been able to implement this feature myself :)

javiereguiluz added a commit that referenced this pull request Feb 23, 2018
This PR was merged into the 1.17.x-dev branch.

Discussion
----------

Tweaked the new form tabs feature

Minor changes for #1887 and added a test.

Commits
-------

c52c376 Tweaked the new form tabs feature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants