Skip to content

Conversation

@max-nextcloud
Copy link
Collaborator

  • Target version: master (after the Nextcloud 24 fork)

Summary

In order to publish an npm package
and reuse parts of the text code in collectives
split the js source into

  • src - for all things text specific, in particular webpack entries
  • package - for the package, that is yet to be defined in package.json.

@max-nextcloud max-nextcloud force-pushed the refactor/npm-package branch 2 times, most recently from e645126 to 9e808fd Compare April 5, 2022 12:44
Copy link
Member

@vinicius73 vinicius73 left a comment

Choose a reason for hiding this comment

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

There are some eslint false positives.

@max-nextcloud
Copy link
Collaborator Author

max-nextcloud commented Apr 5, 2022

There are some eslint false positives.

Yes... i am wondering how to tell eslint that the files are available in the package dir.

Figured it out. The eslint node extension does not know how to use resolvers. So we must disable it's warnings to only use the eslint import extension which uses resolvers just fine: 5adf417

name: 'ViewerComponent',
components: {
EditorWrapper: () => import(/* webpackChunkName: "editor" */'./EditorWrapper'),
EditorWrapper: () => import(/* webpackChunkName: "editor" */'@nextcloud/text-editor'),
Copy link
Member

Choose a reason for hiding this comment

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

We can move it to a local file and re-export it as a local component.
It will turn easier to adjust lazy load and other possible adjustments.

const EditorWrapper = () => import(/* webpackChunkName: "editor" */'@nextcloud/text-editor')

export default EditorWrapper

Copy link
Member

Choose a reason for hiding this comment

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

Also, we can access any exported value inside @nextcloud/text-editor.

const EditorWrapper = () => import(/* webpackChunkName: "editor" */'@nextcloud/text-editor')
  .then(package => package.default)

It is important because we are using named exports like store

import { store } from '@nextcloud/text-editor'

Lazy loading the default export and do not use lazy loading in named exports are incompatibles.

To make it, we must use lazy loading in named imports too.

const Store = () => import(/* webpackChunkName: "editor" */'@nextcloud/text-editor')
  .then(package => package.store)

But it does not make a lot of sense.
In our architecture, we can't use the async store without changing some parts of code and flow.

To make it possible we need to split the store and components into different parts/files of the @nextcloud/text-editor package.
It is possible, but I don't know if webpack will handle it well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well... it's possible - the store for example already is in a file of its own and importing just that file worked fine. I also noticed that highlight.js also uses paths into the package as more fine grained imports. So that seems to be an somewhat established option. Not pretty but maybe enough to get a first working iteration.

I might just use the file paths into the alias in src and then index.js to define the exports of the package that we can then use in collectives. I think there we just want the tiptap extensions for displaying the tables etc. nicely.

We might also want the editor wrapper to get rid of the magic we use to extract the editor component from the viewer. But that could be a separate step or we could just reference it by path to avoid bloating the read only editor in collectives if we include the EditorWrapper in index.js.

@max-nextcloud max-nextcloud force-pushed the refactor/npm-package branch from dcb3779 to b57535a Compare April 7, 2022 07:16
@max-nextcloud
Copy link
Collaborator Author

max-nextcloud commented Apr 7, 2022

Open questions:

  • how do we call the package / alias? @nextcloud/text-editor for now.

Next steps:

  • create npm package (at least locally)
  • use RichText extension to build the preview
  • include css
  • include css from text in collectives rather than having a copy
  • allow customizing link handling in text.
  • handle links inside a collective in collectives app.
  • ensure all other links still work properly.
  • publish 0.1.0 of the package.

Discarded:

  • use EditorWrapper component to build editor in collectives rather than taking it from the view handlers
    If we compiled the editor into collectives it would differ from the one in text on older nextcloud instances.
    This way you could create tables in collectives - but they could not be rendered in text.
    It would also not be possible to share a session between the two.

@max-nextcloud max-nextcloud force-pushed the refactor/npm-package branch 3 times, most recently from 8c6cc0c to ff554d0 Compare April 13, 2022 06:44
@juliusknorr juliusknorr added enhancement New feature or request 2. developing labels Apr 20, 2022
@blizzz blizzz added this to the Nextcloud 25 milestone Apr 21, 2022
@max-nextcloud max-nextcloud force-pushed the refactor/npm-package branch 4 times, most recently from 8440c11 to 4ece1b9 Compare April 27, 2022 09:52
In order to publish an npm package
and reuse parts of the text code in collectives
split the js source into
* src - for all things text specific, in particular webpack entries
* package - for the package, that is yet to be defined in package.json.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
eslint-plugin-import will use webpack resolver and check for missing imports.
The node variant is causing false error reports as it does not use resolvers.

See mysticatea/eslint-plugin-node#249 (comment) for details.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Introduce a `Plaintext` tiptap extension
that bundles all the extensions used for plain text editing.

This allows calling `Editor.new` directly with just a few extensions.
No need to rely on `createEditor` from the editor factory anymore.
Also prevent error messages about undefined callbacks
because no callbacks were handed to `createEditor` in `ReadOnlyEditor`.

Signed-off-by: Max <max@nextcloud.com>
The Prosemirror plugin with the `handleClick` handler
only customizes the prosemirror handling of the click event.
In read only mode we are not in a content-editable section.
So clicking a link will cause the browser to open the url with a page reload.

Allow overwriting this behavior by handling all link clicks via prosemirror.
Set `onClick` option on the `Link` mark to customize the behavior.

Emit a `click-link` event from `ReadOnlyEditor` with info about the event
and the attributes of the link mark.

Find the link that was clicked based on the clicked marks
rather than the element in the event.
This way we can get access to the attributes of the mark
without relying on the selection or even changing it.

Also add plugin key to link click handler

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
* Resolve the position given to the `handleClick` function.
* Get the `link` mark from the resolved position.
* Use the attrs of this mark.

`editor.getAttributes` gets the attributes at the current selection.
In Firefox the selection will not move to the clicked position in read only mode.
So instead of relying on the selection we now use the `pos` of the click.

Signed-off-by: Max <max@nextcloud.com>
If no session is established we cannot use the text endpoint.
Use the core preview or the file_share publicpreview endpoints instead.

This requires the fileId of the text file to be injected
so the relative path to the image inside the attachments folder
can be computed.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Public Preview only has one route - it does not make use of the fileId.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud max-nextcloud mentioned this pull request May 12, 2022
7 tasks
@max-nextcloud max-nextcloud deleted the refactor/npm-package branch May 13, 2022 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing enhancement New feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants