-
Notifications
You must be signed in to change notification settings - Fork 35
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
VM Status notification and Compile Button UI #80
VM Status notification and Compile Button UI #80
Conversation
…o notify user when the compile button is ready. Changes in sys-view-model.js: * Added two default configs for notific8 - one for warnings and other for successful ones. * Modified the computed functions for compile button and error labels to add the notific8 calls. Changes in index.html: * Added the library jquery-notific8 to add notifications in sysbuild Closes cs-education#75
…o notify user when the compile button is ready. Changes in sys-view-model.js: * Added a default config for notific8 - for busy state. * Modified the computed functions for compile button and error labels to fix an unwanted notification during compiling. Changes in index.html: * Added the css library jquery-notific8 to add notifications in sysbuild. Closes cs-education#75
…o notify user when the compile button is ready. Changes in sys-view-model.js: * Fixed formatting issue Closes cs-education#75
@@ -39,7 +57,13 @@ window.SysViewModel = (function () { | |||
return 'label label-' + compileStatusToLabelClassMap[self.compileStatus()]; | |||
}); | |||
self.compileBtnEnable = ko.pureComputed(function () { | |||
return !(self.compileStatus() === 'Waiting' || self.compileStatus() === 'Compiling'); | |||
var status = (self.compileStatus() === 'Waiting' || self.compileStatus() === 'Compiling'); |
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.
minor quirk: status
can be renamed to ready
and set to the negation of the expression on the right, instead of negating twice below.
I like this change! I think the popup could be on the screen longer and the animation could be slower. (maybe a lifespan of ~5 seconds). Eventually we could add a user preference to disable the popups altogether. I agree that this doesn't quite satisfy the criteria of #75. While it makes the state of the compiler much more obvious, it doesn't actually make the play button any more obvious. Suggestion: move play button into editor itself, make it bigger, and make it somewhat translucent when not hovered over. See rough example below: |
I get your point now, I'll try making it like a floating button where it kind of keeps changing it's position based on the position of the mouse cursor. That could be an interesting UX. Any thoughts on this? Meanwhile I'll search if there are plug-ins that make it easy and smooth to do such a thing. |
What do you mean by a floating button that keeps changing position? |
Also, while we are at this, do you think it is a good idea to encapsulate notifications into another module, which then provides an API to other modules? It could contain notification settings, history of notifications displayed, etc. |
@scowalt @anant-singh how does this look? Move the gcc args and program args box on top of each other, and make the play button span two lines in height. It makes the play button bigger but everything remains the same. |
@neelabhg I could see your idea working as well. I think having the notifications in a separate module is a good idea, but potentially something for a different pull request considering the issue this one is trying to address. |
@neelabhg I also thought of doing the same about the notification module, should we push this into another issue or just modify the description of this one. |
As for the floating button concept, I may have not been able to clearly explain it last time. It was an extension to scott's idea of moving in to the editor. But instead of just specifying one static place for the button, it would be in line with the line the user is currently editing like in the example screenshot. This way the compile button would always be in focus near the area where the users' attention currently lies. For it's x-coordinate we could use the white line in the editor, since nothing is supposed to be written beyond the white line anyways. As for when the editor is out of focus or the button is not ready it can just go back to its original place |
Changes : * Incorporated suggestions since the last commit. * Increased the notification duration. * Compile button now spans over two lines with an added text "Run it" on it. * Changed some CSS elements in sysplayground.scss. Closes cs-education#75
Changes : * Minor error fix Closes cs-education#75
Changes : * Minor css adjustment for the compile button Closes cs-education#75
This is great. I think #78 will also help to make the button more clear, so I'm fine with the play button as it is in this commit. |
Superb. Great job @anant-singh! And thanks for the review and discussion @scowalt. |
VM Status notification and Compile Button UI
Changes in sys-view-model.js:
Changes in index.html:
Closes Make play button more obvious #75