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

TASK: only show button when enabled #8

Merged
merged 2 commits into from
Mar 8, 2024

Conversation

PRGfx
Copy link
Contributor

@PRGfx PRGfx commented Jan 9, 2024

This PR creates a new build of the plugin in order to only show the button when the feature is enabled in the node-type configuration.

Changes in this PR

As discussed in #6, this PR updates the build-stack to the current UI-extensibility setup:

  • use new @neos-project/neos-ui-extensibility stack with esbuild
  • remove dependency on plow-js
  • use provided ckeditor-exports rather than installing the @ckeditor
    this resolves a runtime error of having different ckeditor versions
  • add lock-file
  • restrict package to newer Neos versions

How to verify

  • Configure SplitAdd for a single node-type (e.g. Text)
    'Vendor.Package:Content.Text': 
      properties:
        text:
          ui:
            inline:
              editorOptions:
                formatting:
                  splitAdd: true
    click into another inline-editable node-type (e.g. a Heading) and into the configured node-type and verify that the Button is in the RTE toolbar only for the node-type with the setting
  • Verify that the button still triggers node creation

Open questions

  • What can/should we restrict the Neos version to? I only got to test it on 8.2, but I expect that it should work on older versions as well. The relation to the neos-ui-extensibility version is not obvious to me. The ckeditor-exports import might have been breaking at some point...

resolves #6

- use new `@neos-project/neos-ui-extensibility` stack with esbuild
- remove dependency on `plow-js`
- use provided `ckeditor-exports` rather than installing the `@ckeditor`
  this resolves a runtime error of having different ckeditor versions
- add lock-file
@PRGfx PRGfx marked this pull request as draft January 9, 2024 17:35
composer.json Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
@mhsdesign
Copy link
Contributor

Ill test this in 7.3 and 8.3 :)

Co-authored-by: Marc Henry Schultz <[email protected]>
Copy link
Contributor

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

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

Thanks for the update ;) It looks all good and works as expected. I could also confirm that the option splitAdd was not used previously.

  properties:
    text:
      ui:
        inline:
          editorOptions:
            formatting:
              splitAdd: true

So in the release notes - its still beta so all fine - it should be noted that this option is now always required to be used.

composer.json Show resolved Hide resolved
Copy link
Contributor

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

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

Thank you for updating the tech ;)

I can confirm that this works with Neos 8.3
Well at least how good the plugin ever worked ... i found a few bugs #9
but they are unrelated to this change.

@mhsdesign
Copy link
Contributor

as stated here #8 (review) please mark this change as breaking and add as upgrade note that you have to enable it ;)

  properties:
    text:
      ui:
        inline:
          editorOptions:
            formatting:
              splitAdd: true

(cant edit messages on foreign repos ... dang ^^)

@lorenzulrich
Copy link
Collaborator

@mhsdesign So you suggest that this is merged and released as a new major version, correct? I would then merge and release.

@lorenzulrich lorenzulrich marked this pull request as ready for review March 8, 2024 08:20
@lorenzulrich lorenzulrich changed the title DRAFT: TASK: only show button when enabled TASK: only show button when enabled Mar 8, 2024
@lorenzulrich
Copy link
Collaborator

@PRGfx Thanks for your contribution!

@lorenzulrich lorenzulrich merged commit 4d68cda into psmb:master Mar 8, 2024
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.

Button is always visible / JS build seems out of date
3 participants