-
Notifications
You must be signed in to change notification settings - Fork 18
feat: show loading screen in task area only #256
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
Conversation
93ca8a5
to
a2f2166
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.
This looks great! A few things to adjust and we're good to go.
The task list item is not updated when the task finishes. Maybe we should fire an event in the event bus after pollTask so the TaskList component can update its internal task object status.
emit('task-updated', task)
which would be caught in TaskList and call a method like:
updateTask(task) {
const internalTask = this.tasks.find(t => t.id === task.id)
if (internalTask) {
internalTask.status = task.status
}
}
Is Another alternative could be instead of declaring the tasks list in the |
It's a lot of chaining props to carry the value from assistant.js to TaskList. I would be in favor of using the event bus. When using this function: |
bcdb3d1
to
08c1af1
Compare
done in 08c1af1, but using |
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.
Works great. @janepie Could you give it a try as well?
@edward-ly Let's wait for Jana's feedback to merge this if you don't mind.
Nice one! |
@edward-ly You can rebase your branch on main where part of the task type switch issue was fixed (by #262). |
08c1af1
to
12ecdad
Compare
Good catch, should be fixed in 12ecdad now, mind testing one last time? @janepie @julien-nc |
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.
The issue with switching task types while a task is "loading" is fixed when opening the assistant with the menu entry but not when opening it from a notification. See inline comment.
12ecdad
to
1005bd3
Compare
Not necessarily the red color, but I do see the button being in the wrong state too, latest commit should fix it. @janepie feel free to merge this if everything looks good to you |
…tion property Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
… click, toggle notification request instead Signed-off-by: Edward Ly <contact@edward.ly>
…ng screen depending on task status Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
b7ec048
to
d3110c3
Compare
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
d3110c3
to
d03bead
Compare
Ah I see, I fixed the case for when a task from the task list is loaded, but not when switching task types or creating a new task, should be fixed now. |
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 good, really nice work!
* Adjust `get notified` string. * Check for changed icon rather than message. Changes caused by nextcloud/assistant#256 Signed-off-by: Max <max@nextcloud.com>
* Adjust `get notified` string. * Check for changed icon rather than message. Changes caused by nextcloud/assistant#256 Signed-off-by: Max <max@nextcloud.com>
* Adjust `get notified` string. * Check for changed icon rather than message. Changes caused by nextcloud/assistant#256 Signed-off-by: Max <max@nextcloud.com>
Fixes #245. In addition, the task list is automatically updated when a new task is submitted or a running task is canceled, with the submitted task also being automatically selected from the list.