-
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
Text processing integration #4627
Conversation
a3ffbd8
to
5d27aa0
Compare
6387d31
to
525faf5
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.
Looks really great! :) The only thing I am a bit worried about is that even with the icon on the button, "Show assistant results" in the menu then is not super obvious.
- Should we permanently show "Assistant results" as an entry on the top instead?
- We could also say "Assistant results (4 new)" or have a counter bubble there for unviewed results if possible?
- Other ideas?
Looks really nice! Super cool! Only one thing: maybe we can add a separator line before "Show assistant results" to indicate a different type of action. Other than that it looks great! :) |
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
I think the indicator about the status already gives an idea, considering the timeline of getting this merged, I'd rather get it in and align during the demo next week if this is something we can polish next week and delay screenshot taking or keep as it is. In any case having clear mockups for any permanent display option would be appreciated then. |
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 looks good. Did not try myself.
I think I'd personally compose the Assistent.vue
out of smaller components for the Floating Menu and the Modal. But that can also happen later. I also did not check how much code reuse there is between the two. If they overlap a lot might not be worth it either.
Signed-off-by: Julius Härtl <[email protected]>
525faf5
to
5ebcd8b
Compare
/compile |
Signed-off-by: nextcloud-command <[email protected]>
/backport to stable27 |
The backport to stable27 failed. Please do this backport manually. # Switch to the target branch and update it
git checkout stable27
git pull origin stable27
# Create the new backport branch
git checkout -b fix/foo-stable27
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123
# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27 More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport |
/backport 526302d,130a4ec5fe5f24e0e27a63e459eeea22472770e5,5ebcd8bd7788afa300b2da098a88067b59eec9dc to stable27 |
The backport to stable27 failed. Please do this backport manually. # Switch to the target branch and update it
git checkout stable27
git pull origin stable27
# Create the new backport branch
git checkout -b fix/foo-stable27
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123
# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27 More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport |
📝 Summary
🚧 TODO
Follow ups
🖼️ Screenshots
🏁 Checklist
npm run lint
/npm run stylelint
/composer run cs:check
)