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.

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

[FEATURE] Notify user if there is a new code remark and link to Dodona #62

Closed
BTWS2 opened this issue Oct 6, 2020 · 8 comments
Closed
Assignees
Labels
feature ⭐ New features
Milestone

Comments

@BTWS2
Copy link
Contributor

BTWS2 commented Oct 6, 2020

Bart brought up an interesting point. If there is new feedback available, students don't see the bell icon immediately if they use the extension.

I already had several occasions where I gave feedback on code and students only noticed it 15 minutes (or more) later when programming. Is this feature request possible with the current API?

image

I think there are two ways (or both) to solve this problem (and should be in alignment with the vision of Bart because it sends users to the Dodona website):

image image

@BTWS2 BTWS2 added the feature ⭐ New features label Oct 6, 2020
@stijndcl
Copy link
Collaborator

stijndcl commented Oct 6, 2020

This does sound like a really good idea, I hadn't thought of the Dodona notifications yet. I just looked around a bit and it looks like it should be doable. This could run as a background task that checks Dodona every few minutes (doing it constantly would be really bad performance-wise), considering notifications on Dodona are not very common.

stijndcl added a commit that referenced this issue Oct 6, 2020
stijndcl added a commit that referenced this issue Oct 6, 2020
…ground

Don't check only when the plugin is open (so students always get notified, not only when working for Dodona)
Add a bell icon to link to notifications
Fixes #62
@stijndcl
Copy link
Collaborator

stijndcl commented Oct 6, 2020

I've implemented everything except the dynamically changing bell icon. Not quite sure on how I'm gonna do that yet, but the information message bottom-right is already a thing at least (and the bell can be clicked to open notifications).

After looking around I didn't find any useful information on how to make the ViewAction's when clause change dynamically from code, or after triggering something (listening to variables, ...).

@BTWS2
Copy link
Contributor Author

BTWS2 commented Oct 6, 2020

Thank you! Is a 30 second interval doable for Dodona? If 500 to 1000 students use VS Code simultaneously will it still work without issues? The time of the interval is the worst-case scenario, so an interval of 1 or 2 minutes is also ok for me. Notifications don't happen that often.

I did some research and this is what I found, but I don't know if it's helpful:

@stijndcl
Copy link
Collaborator

stijndcl commented Oct 7, 2020

I'm not sure, I'll ask someone from Dodona what type of interval they'd be most comfortable with. I went with 30 seconds because my initial idea was a few minutes to increase performance, but I found out it only takes a few ms to do it & it was really inexpensive.

Also thank you for the SQL source link, I'm sure there's something in there that could be of use.

I've seen that Stackoverflow article before I think, when we were trying to dynamically update the exercise icons from wrong to correct. It goes over how to change a tree view item's icon (which, turns out, you can't really do, so we had to use a bit of a hacky way), which is loaded from code instead of the package.json file.

@BTWS2
Copy link
Contributor Author

BTWS2 commented Oct 7, 2020

Adding this for reference and might be useful: dodona-edu/dodona#2302

@stijndcl
Copy link
Collaborator

stijndcl commented Oct 7, 2020

Where would you put this? I don't immediately see a place where this would fit in. The only thing I can think of is updating the Dodona icon in the sidebar, but that poses the same issue as changing the bell icon. If I knew how to fix either, I'd implement both.

@BTWS2
Copy link
Contributor Author

BTWS2 commented Oct 7, 2020

It was mainly for the fact that Dodona only checks the status of the notifications once every minute. They also show how many, but for the extension showing if there are notifications should be enough.

@stijndcl
Copy link
Collaborator

stijndcl commented Oct 7, 2020

Ah, I thought you referenced the PR itself instead of the one mentioned lower. In that case I'll go for every minute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⭐ New features
Projects
None yet
Development

No branches or pull requests

3 participants