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

Replaces the admin form builder grid and forms #4480

Merged
merged 34 commits into from Sep 21, 2017

Conversation

Projects
None yet
2 participants
@guerler
Contributor

guerler commented Aug 22, 2017

This PR replaces the admin formbuilder grids with client-side rendering modules.

guerler added some commits Aug 22, 2017

@guerler guerler modified the milestones: 18.01, 17.09 Aug 30, 2017

@guerler guerler changed the title from [WIP] Replaces the admin form builder grid and forms to Replaces the admin form builder grid and forms Aug 30, 2017

guerler added some commits Aug 31, 2017

@guerler guerler added status/review and removed status/WIP labels Sep 13, 2017

@dannon

This comment has been minimized.

Member

dannon commented Sep 21, 2017

This looks good and works well in my testing.

One bit of feedback that can hopefully be addressed in future work is that I preferred the old flow during form creation. In the old one, when you make a form, you are shuffled towards immediately adding fields as a part of the creation process. With this new form, upon creation (prior to adding any fields), it kicks you back to the form list, and from there you have to click back in to edit the form.

It'd be nice to be able to create the form and add the fields all in one step, as before.

That said, still a big step forward and I'm going to merge this now to avoid more merge pain :)

@dannon dannon merged commit c9db5f8 into galaxyproject:dev Sep 21, 2017

5 of 6 checks passed

toolshed test Test started.
Details
api test Build finished. 292 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 161 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 46 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment