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

Enhancement: SilverStripe 4 compatibility #137

Merged
merged 3 commits into from
Jan 25, 2017

Conversation

robbieaverill
Copy link
Contributor

@robbieaverill robbieaverill commented Jan 17, 2017

Enhancement: SilverStripe 4 compatibility

This is a work in progress, but provides a usable start for a 4.x upgrade.

There is still some work that needs to be done on the user interface. Some things (like the edit pencils on the GridField rows) appear to have been replaced in the SS4 framework, and bootstrap theming should now be applied everywhere. I've started this for the BulkManager.

Summary

  • Implemented PSR-4 autoloader for each of the BulkManager and BulkUpload sections of this module
  • Added and implemented namespacing throughout
  • Adjusted framework API changes as required (e.g. SS_HTTPRequest -> HTTPRequest)
  • Applied some basic bootstrap styles to the BulkManager GridField actions
  • Update composer requirements
  • Update documentation to include namespacing

Contributes towards #136 but doesn't bring it quite close enough to "stable" to resolve it.

@colymba
Copy link
Collaborator

colymba commented Jan 17, 2017

Thanks @robbieaverill! A quick thought.. would it make sense to clean up all those class names now that we have namespacing? I always thought GridFieldBulkActionDeleteHandler for example was overly long, and this would give us the opportunity to clean/tidy up those names...

Something like....
GridFieldBulkActionDeleteHandler: Colymba\BulkManager\BulkAction\DeleteHandler
GridFieldBulkActionEditHandler: Colymba\BulkManager\BulkAction\EditHandler
GridFieldBulkActionHandler: Colymba\BulkManager\BulkAction\Handler
GridFieldBulkActionUnlinkHandler: Colymba\BulkManager\BulkAction\UnlinkHandler
GridFieldBulkManager: Colymba\BulkManager\BulkActionManager

BulkUploadField: Colymba\BulkUpload\BulkUploadField
GridFieldBulkImageUpload: [deprecated]
GridFieldBulkUpload: Colymba\BulkUpload\BulkUploader
GridFieldBulkUpload_Request: Colymba\BulkUpload\BulkUploaderRequest

what do you think?

@robbieaverill
Copy link
Contributor Author

@colymba - yeah, I think it makes sense to shorten the names where you can. Will update now.

@robbieaverill
Copy link
Contributor Author

It's worth noting here that the bulk upload form is possibly not working. If I have some time I will look into adjusting it, but I don't imagine it will take much work. The bulk manager seems to be OK, and both parts are rendering on page load.

They'll both likely need some styling tweaks to suit the new bootstrap theme in the CMS.

@robbieaverill
Copy link
Contributor Author

Hey @colymba - how do you feel about the updated version?

@colymba
Copy link
Collaborator

colymba commented Jan 23, 2017

@robbieaverill sorry am not following up more actively.... I feel good about the update... I need to test it, but this will be good for the module no matter what.

I had a question about the build.xml file, I saw this got deleted... Phing is probably old school now and I always wanted to replace it with a Gulp or Grunt task. And since SS4 is now using npm too, should we just delete the tasks folder altogether and I'll upgrade this later?

@robbieaverill
Copy link
Contributor Author

Hey @colymba - sorry, I'm not overly familiar with the system, but @tractorcow mentioned it was not used any more so I figured it could be deleted

@colymba
Copy link
Collaborator

colymba commented Jan 24, 2017

Hey @robbieaveril, I've done some local testing and the switch to SS 4 needs a lot of cosmetics :) The BulkManager seems to basically work, pending more in depth testing, which is great.

The BulkUploader on the other hand, like you said earlier, needs a lot of work. I've got a crude version working for basic upload. Getting this to work also involves switching to https://github.com/silverstripe/silverstripe-asset-admin which I believe the Framework is doing (silverstripe/silverstripe-framework#6481).

@robbieaverill
Copy link
Contributor Author

Hey @colymba - yeah. To be transparent, my reasons for this pull request are that I need to upgrade modules that depend on this one :) this is why I've sort of got it to a usable state without trying to make it perfect.

Re: the uploader, you will probably need to make some changes for the asset abstraction that has been implemented (see here).

@tractorcow
Copy link
Contributor

@colymba we're building a new i18n / text collection system in 4.0; Might be worth looking at porting this module over to using cow. https://github.com/silverstripe/cow/tree/1#module-level-commands

@colymba
Copy link
Collaborator

colymba commented Jan 25, 2017

thanks @tractorcow I'll be sure to take a look and see what I can do.

@robbieaverill I'll merge this and push another commit for the BulkUploader so we have everything in some sorts of working order. Then I'll work on polishing it.

@colymba colymba merged commit 7221897 into silverstripe:master Jan 25, 2017
@tractorcow
Copy link
Contributor

Also check out silverstripe/silverstripe-framework#6558; You can probably write something symfony specific if you want to roll your own text collector / localisations. :)

@tractorcow tractorcow deleted the feature/ss4-compat branch January 25, 2017 22:54
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