-
Notifications
You must be signed in to change notification settings - Fork 91
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
feat: bring back cypress command createDescription through UI #4965
Conversation
@@ -144,9 +143,10 @@ export default { | |||
}) | |||
}, | |||
getFileInfo(autofocus) { | |||
if (!this.hasRichWorkspace) { |
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.
Can you elaborate a bit why this change is needed? as far as I see this would no longer check on each folder change on if the folder has a rich workspace
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.
As I checked,
- when Readme.md file was created, it did not trigger updated function (at https://github.com/nextcloud/text/blob/main/src/helpers/files.js#L232) to update property hasRichWorkspace's value to true.
- So, the RichWorkspace component was not be shown even though Text::showRichWorkspace was triggered since the property hasRichWorkspace was still false (until page refreshed)
src/helpers/files.js
Outdated
setTimeout(() => { | ||
emit('Text::showRichWorkspace', { autofocus: true }) | ||
}, 200) |
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.
Can you tell why the previous one didn't work?
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.
When user directly accessed an empty folder, the render() (at https://github.com/nextcloud/text/blob/main/src/helpers/files.js#L207) did not run. So, RichWorkspace component was not created.
Then, triggering Text::showRichWorkspace
right after creating Readme.md did not work.
The RichWorkspace component was created only after any file or folder was created.
So, we have to wait some time until RichWorkspace component is created.
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.
Ah i see, that seems tricky. Let's get this in then for now. We might want to have the files app adjusted so that the header component itself is also rendered for empty file lists mid term. I only pushed a commit to use the file created event instead which seemed more clean.
@@ -12,5 +12,6 @@ registerDavProperty('nc:rich-workspace', { nc: 'http://nextcloud.org/ns' }) | |||
registerDavProperty('nc:rich-workspace-file', { nc: 'http://nextcloud.org/ns' }) | |||
|
|||
if (workspaceAvailable) { | |||
addMenuRichWorkspace() |
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.
Good idea to move that over 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.
Would be great to get a bit of further insights on the changes (see inline)
9f16fd3
to
f73bd2f
Compare
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.
Code changes look good to me 👌
Signed-off-by: Luka Trovic <[email protected]>
Signed-off-by: Luka Trovic <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
This will ensure that we can create the workspace in the new file menu plugin and show it without requiring another propfind/reload (which is not available through the new files app apis). Once the file list is rendering the header we can use the flag to properly indicate the state to the file header plugin Signed-off-by: Julius Härtl <[email protected]>
f73bd2f
to
2cfa5e4
Compare
@luka-nextcloud just FYI I found a reasonable workaround for updating the view without the timestamp (see last commit) |
📝 Summary
🖼️ Screenshots
🚧 TODO
🏁 Checklist
npm run lint
/npm run stylelint
/composer run cs:check
)