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

Move import form to separate page #209

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tautelis
Copy link

Q A
Bug fix? no
New feature? yes
BC breaks? yes in UI, no in CLI
Fixed tickets #208 #71

Main goal of this PR:

  • move import form to separate import page so the index page is not cluttered

Improvements:

  • having import button in top is intuitive when coming from other e-commerce platforms
  • most of the import form data validation is moved from Controller to ImportType
  • JS script added to display the name of the selected file for upload
  • since this will be a distributable package import route was prefixed with fos_sylius_import_export_ to avoid clashing with any project level routes prefixed with app_
  • importer tag types replaced with full sylius resource name same as exporter tags to keep the consistency and avoid sylius resource name ambiguity

UI import before:
index-page-before

UI import after:
index-page-after

import-page

@tautelis tautelis force-pushed the issue_208/move_import_form_to_separate_page branch from c5b4cf5 to 4152e62 Compare November 11, 2019 07:56
@tautelis tautelis force-pushed the issue_208/move_import_form_to_separate_page branch from 4152e62 to fa474fa Compare November 11, 2019 08:07
@oallain oallain added this to the 1.0.0 milestone Nov 29, 2019
@oallain oallain self-assigned this Nov 29, 2019
@tautelis
Copy link
Author

well, the rule of thumb is if you keep the PR open for too long the conflicts appear... why are we waiting this much? is there any problem with this PR? Things like this demotivates to contribute to open source projects.

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.

2 participants