-
Notifications
You must be signed in to change notification settings - Fork 155
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
[full-ci] Improvements to text/md editor #6667
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
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.
Nice start, I have some hints for improvements, but I like where this is going :)
@dschmidt thanks for your feedback, it helped me implementing what was missing as well.
I've also renamed it from markdow-editor to simple-editor. I have the impression this naming is clearer for normal users, but if you prefer I can revert that commit. What is missing is an option to disable md creation, since we would prefer to do it with codimd. I might need another iteration with the tests. |
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.
Much better! I like!
...fileExtensionConfig | ||
}) | ||
} | ||
|
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.
Not a fan of this, maybe we can register editors for mimetypes in the long run?
As we probably don't have a mechanism for that currently, it's totally out of scope for this PR. Maybe you can open a ticket?
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.
Actually, I built registering apps on mime types in #6514 but that one got stale...
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.
👍 for that! I would like to be able to open files with no extension.
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 hope you don't get too overexited with this. Turns out that mime type guessing from the backend isn't always what you'd expect. E.g. ogg video files still get announced with audio/ogg
as mime type. But I continued the PR and it's close to being merged.
@@ -17,8 +19,8 @@ const fileExtensionConfig = { | |||
} | |||
|
|||
const appInfo = { | |||
name: 'MarkdownEditor', | |||
id: 'markdown-editor', | |||
name: 'SimpleEditor', |
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.
totally fine with the rename, it's scoped way better than markdown-editor
. 👍
Results for oC10Files2 https://drone.owncloud.com/owncloud/web/24564/18/1 |
@@ -78,7 +78,7 @@ config = { | |||
"oC10Files1": [ | |||
"webUIFilesCopy", | |||
"webUIFavorites", | |||
"webUIMarkdownEditor", | |||
"webUISimpleEditor", |
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.
@diocas that would need a rename of https://github.com/owncloud/web/tree/master/tests/acceptance/features/webUIMarkdownEditor in order to work :) since it's mapping feature files/folders
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.
ups
@diocas could you search&replace various combinations of the word |
Support for read-only files Disable preview for non md files (configurable) Data integrity checks (prevent exiting with unsaved changes) Better error information to the user Keyboard shortcut to save
Enable text editor for other extensions
I actually did find&replace, but it took me some time to realize that I filter some folders when I'm in dev mode... Should be fine now, but let's see how the tests go. |
Btw, what about the disabling of md file creation? |
What do you mean? |
Can you open an issue please? |
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.
Found a typo. Fixing that should finally turn the test suite green 🥳
tests/acceptance/features/webUIFilesActionMenu/fileFolderActionMenu.feature
Outdated
Show resolved
Hide resolved
…nMenu.feature Co-authored-by: Benedikt Kulmann <[email protected]>
SonarCloud Quality Gate failed. |
I'm trying to push upstream some improvements that we have in our own text-editor so that we can drop it. This PR is to discuss the direction and to have some pointers on how to implement things (sorry, I'm not well versed in Vue, and even less in composition api).
What this PR adds:
Still in early stages, I'm also trying to make the app configurable, because we might want to remove the preview pane, since we also want to disable the markdown support (check #6661 )