Conversation
|
|
| refetchButton.setAttribute('title', 'If you broke any of your assignment files and you want to redownload them, delete those files and click this button to refetch the original version of those files.') | ||
| container.append(refetchButton); | ||
|
|
||
| refetchButton.innerText = 'Refetch'; |
There was a problem hiding this comment.
"Refetch" is quite ambiguous, somebody might expect that it will overwrite existing files. Can we call it "Fetch missing files" (or "Refetch missing files")?
There was a problem hiding this comment.
It would also be nice to have the button disabled if no files are actually missing.
There was a problem hiding this comment.
Sure, the button can be renamed to "Refetch missing files".
The problem with disabling the button is that the Exchange has no API for listing all the files of an assignment. Or well... there is ExchangeList.list_files(), but in the default exchange, it is implemented in a weird way, where it does not really list any files. Instead, it returns a list of assignments. So it sure can be done by modifying the exchange a bit, but people can already rely on the way it works now so then it would break their stuff. Someone can even have their own exchange implementation, and since the output of list_files() is not unified, I think, it will not work for everyone.
Do you have any better idea regarding this?
There was a problem hiding this comment.
Oh, so the API would probably have to be extended with a new function... But it can be done later, this is still a big improvement!
There was a problem hiding this comment.
Oh, so the API would probably have to be extended with a new function... But it can be done later, this is still a big improvement!
.... also need to factor in that any plugin exchange systems would need to mimic that functionality..... and I know that our nbexchange service has no list of files in an assignment, just the list of .ipynb files
There was a problem hiding this comment.
Thanks @KrKOo for working on it.
I agree with @lahwaacz that it could be nice to disable the button if not needed, but understand that it is not easy at the time. Let's keep it for an other PR.
I have some comments below, and also wonder if we can add some tests on this new feature. I can help on it if necessary.
| } | ||
| } else if (this.data.status == 'fetched') { | ||
| var refetchButton = document.createElement('button'); | ||
| refetchButton.classList.add('btn', 'btn-danger', 'btn-xs'); |
There was a problem hiding this comment.
The class btn-danger does not seem to be used.
In this case I would call it btn-warning, since this is not supposed to overwrite anything.
And the style of this class could be defined in the common.css file, like btn-default and btn-primary.
| refetchButton.setAttribute('data-bs-toggle', 'tooltip'); | ||
| refetchButton.setAttribute('data-bs-placement', 'top'); |
| refetchButton.classList.add('btn', 'btn-danger', 'btn-xs'); | ||
| refetchButton.setAttribute('data-bs-toggle', 'tooltip'); | ||
| refetchButton.setAttribute('data-bs-placement', 'top'); | ||
| refetchButton.setAttribute('style', 'background:#d43f3a; margin-left:5px'); |
There was a problem hiding this comment.
As said above, the style properties could be in common.css.
I would also suggest:
- an orange background
- the same font color as the neighbouring button
- slightly change the background color on hover
4 participants
This PR adds a "Refetch" button to the student's assignment list. This allows the student to reset any assignment files to their original version in case they got bricked by the student. Until now, this was possible only using the terminal, which might not be intuitive for some students.
If a student wants to reset a file to its original version, he deletes it and clicks the "Refetch" button. The missing files will be downloaded.