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.

Sign up for GitHub

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

Jump to bottom

Work around OCA.Viewer.file not set when editing in richworkspace mode. #2163

Closed
wants to merge 3 commits into from
Closed

Work around OCA.Viewer.file not set when editing in richworkspace mode. #2163

wants to merge 3 commits into from

Conversation

rotdrop
Copy link

@rotdrop rotdrop commented Feb 7, 2022 •
edited
Loading

@rotdrop rotdrop mentioned this pull request Feb 7, 2022
Closed
@rotdrop
Work around OCA.Viewer.file not set when editing in richworkspace mode.
a1d6ed7 
Signed-off-by: Claus-Justus Heine <himself@claus-justus-heine.de>
@rotdrop rotdrop force-pushed the bugfix/work-around-richworkspace-viewer-file-not-set branch from e56cb15 to a1d6ed7 Compare February 7, 2022 09:06
@rotdrop
2nd-try: fix linked files not working in richworkspace editor.
28d75fb 
Introduce a global property

OCA.Text.RichWorkspaceFilePath

which is used in helpers/links.js if the Viewer-property OCA.Viewer.file
is not available.

This fixes the problem that opening linked files from the enriched
workspace does not work, as OCA.Viewer.file is empty in this case. So
somehow the base MD-file has to be to communicated to domHref() in
links.js.

Signed-off-by: Claus-Justus Heine <himself@claus-justus-heine.de>
@rotdrop
Copy link
Author

rotdrop commented Feb 7, 2022

The second commit removes the extra "context=BASEFILE.md" from the generated links which would modify the Readme.md markdown in an irritating manner.

@juliushaertl juliushaertl added 3. to review bug Something isn't working labels Feb 14, 2022
@juliushaertl juliushaertl added this to the Nextcloud 24 milestone Feb 14, 2022
@juliushaertl juliushaertl self-requested a review February 14, 2022 09:23
@max-nextcloud max-nextcloud self-requested a review March 2, 2022 07:26
@max-nextcloud
Copy link
Collaborator

Hey @rotdrop,
Thanks a lot for the analysis of the proplem and proposed fix. Your fix is perfectly in line with what we did there so far.
However I'd prefer to avoid introducing another global variable that magically switches behavior inside the helper. Using OCA.Viewer inside that helper was a mistake in the first place.

I think we could hand the currentDir to the domHref function in the helper. My understanding is that it's only called from the link mark which should have access to the editor state. We could inject currentDir the way we do it for the Image node.

@max-nextcloud
Copy link
Collaborator

Actually it would probably be better to get this fixed and backported first and clean it up after. Would be good to have a test for the broken behavior to make sure this fixes it and also to prevent it from braking during the refactor afterwards.

@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
Merged
@blizzz blizzz mentioned this pull request Mar 31, 2022
Merged
This was referenced Apr 7, 2022
Merged
Merged
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
This was referenced Aug 12, 2022
Merged
Merged
This was referenced Aug 24, 2022
Merged
Merged
This was referenced Sep 6, 2022
Merged
Merged
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
Merged
This was referenced Sep 20, 2022
Closed
Merged
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@mejo-
Copy link
Member

mejo- commented Nov 1, 2022

Thanks again for the PR @rotdrop. A similar fix got implemented in #3269

@mejo- mejo- closed this Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mejo- mejo- Awaiting requested review from mejo-

@azul azul Awaiting requested review from azul

@juliushaertl juliushaertl Awaiting requested review from juliushaertl

@max-nextcloud max-nextcloud Awaiting requested review from max-nextcloud

Assignees
No one assigned
Labels
2. developing bug Something isn't working
Projects
None yet
Milestone
Nextcloud 26
5 participants
@rotdrop @max-nextcloud @mejo- @blizzz @juliushaertl