-
Notifications
You must be signed in to change notification settings - Fork 6
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
SPLAT-1442 ckeditor5 plugin #107
base: 3.0
Are you sure you want to change the base?
Conversation
arguments: | ||
- '@netgen_remote_media.provider' | ||
calls: | ||
- [setContainer, ['@service_container']] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that all of this setContainer can be replaced by specifying the parent instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've now noticed all of my controllers use SF's AbstractController, whereas all the old controllers do not extend from that. @RandyCupic any reason for that? Should I ditch extending SF's controller, is it maybe unnecessary here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in an IRL conversation we concluded we can remove the AbstractController where it's not needed - and with it, remove the setContainer calls. There was only some reliance in the resource view controller for the template parameter and twig rendering, but both are replaced by direct injections and usage of Twig\Environment and ParameterBagInterface instead of through AbstractController's renderView and getParameter
cc @RandyCupic
2119b7b
…ng and loading ngrm as part of the field
1027868
to
042675f
Compare
@igorkrz can you check out the failing tests please? |
updated the unit tests |
N.B. documentation/installation instructions have not been written yet
Changes
JS plugin
yarn ibexa
)mode: embed
for the extra fields neededendpoints
- most of these are functions that takelocationId
as a parameter, referring to the remote resource locationcreateLocation
updateLocation(locationId)
deleteLocation(locationId)
getSelectedImage(locationId)
- retrieve selectedImage data for the Vue app in the editorviewResource(locationId)
- render a location on the front web, saved as data param. Additionally receive query params:css_class
,variation_group
,variation_name
,alignment
fieldId
- to keep Vue instances on the page uniqueconfig
- Vue config, merged with defaults in the pluginvariationGroup
- necessary for renderingviewEndpoint
alongside some other attributes to get the rendered field from the backendIssues to address
PHP + config
Configuration
config
attribute for the editor, is currently done within project code to fetch it only onceview_resource
- this template is rendered by a controller that is called from JSControllers
All of these have been registered under ajax routes
netgen_remote_media.templates.view_resource
template. Default template in bundle is based on some existing rendering templateServices etc.
Vue
selectedImage
fieldsselectedVariation
- one ofvariations
cssClass
- free text inputmode
for extra fieldsembed
is used to render additional fields that fill in newselectedImage
fieldsngrm-change
event detail has additional infochangedField
property that tells the listener what input caused the event to firemodal
is used for changes triggered by something in a modalname
attributesngrm-change
is firedWebpack
Still needed changes/actions
check for actual uniqueness of the field id when loaded into the editor (currently using command value, can likely cause issues)- donemake- doneendpoints
config partially configurable (fallback to default) similarly to Vue configrecheck- doneconfig
mergeUsage
Using the plugin requires certain node modules to be available so it can be built. This is easily possible when building the editor alongside other project assets because all of those should already be included as dependencies, but if using a pre-built editor (sklepex, might be updated to building the editor with webpack instead of this though), the necessary classes should be exported from the source file and needed aliases set in the webpack config.
Testing
Currently testable on a Sylius project. Can be tested on sklepex where it's configured already for that project - https://bitbucket.org/forlagshuset/sklepex/pull-requests/819
Manual testing requires adding the plugin to the Ckeditor upon initialisation in the project and following the Usage section above, and various configuration information given
Issues
Does not currently work with ibexa - needs some xslt file preparation for ibexa's rich text handling