Skip to content

Conversation

@Stefan9149
Copy link

What is this PR for?

Users may have pending/running tasks and do not want to keep focusing on the web page.
This feature sends a simple notification in the form of blinking web page/tab title, so it can remind users about any updated paragraph with new status FINISHED/ERROR/ABORTED.
Blinking title will recover to original title after user re-focuses on the Zeppelin page.

What type of PR is it?

UI Feature

Todos

N/A

Is there a relevant Jira issue?

No

How should this be tested?

Go into a notebook and input some commands that take some time to finish. Run these paragraphs then leave the page. After getting a blinking page title as finish alert, go back to Zeppelin page to see if there’s a finished paragraph and whether the blinking title recovers.

Screenshots (if appropriate)

part1
part2

Questions:

  • Does the licenses files need update?
    No
  • Is there breaking changes for older versions?
    No
  • Does this needs documentation?
    No

@bzz
Copy link
Member

bzz commented Dec 22, 2015

Great stuff! Page title is important way to communicate to user indeed.

@Stefan9149 There are couple of things you can do to make a contribution be accepted faster i.e for GUI changes it would help to have screenshot or animated gif attached to facilitate a feedback from reviewers.

Another thing that I was also thinking about is - may be we could use the page title as a notification to the user, but start with something less invasive than blinking, i.e as a first step just change the title to show status in text.

What do you guys think? \cc @corneadoug @felizbear @swkimme

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by convention, JS objects and props are camelCased, (e.g. PageTitleNotification -> pageTitleNotification, Vars -> vars)

@prabhjyotsingh
Copy link
Contributor

Could you change the description to follow our PR Template?
(you can find it here )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be if (statusChanged && ($rootScope.windowFocus === false)), otherwise every paragraph will send the event

@corneadoug
Copy link
Contributor

I left a few comments here and there, I also think @felizbear is right about the camelCase.
Overall, the PageTitleNotification part could be done as a service.
For the feature itself, I found out that you never really see the message, depending on your tabs size, it could show just 5-6 characters, so it could be better to have an animated text (just like a prompter).

Didn't try on multiple web browsers yet

@prabhjyotsingh
Copy link
Contributor

I agree to what @corneadoug said; depending on your tabs size, it could show just 5-6 characters. So, you should think of animating text, sound notification, or desktop notification.

Also, say I have more than one paragraph (say 20 of them), and then I click on run all notes; it start showing "You have a job finished!!!" as soon as first paragraph has completed execution.

@Stefan9149 Stefan9149 force-pushed the master branch 2 times, most recently from f1d000d to 453418b Compare January 6, 2016 21:43
@Stefan9149
Copy link
Author

Thanks for all your comments.

I agree with you that blinking tab title is not an ideal way to send notification. I found this project angular-web-notification which makes a wrapper of HTML5 desktop notification and supports for different browsers. I incorporated it into Zeppelin, it works like this:

part1
part2

How do you guys think?

For the "run all paragraphs" issue, indeed we need to find a way to make it as an exception and only send notification when all paragraphs are finished. However I didn't figure out how to keep track of this progress. I can disable notification with "runNote", but I have no idea where/when to re-enable it. And it seems that Zeppelin now supports multi-thread execution within one notebook, so tracking status of last paragraph doesn't work neither. Any suggestion?

@corneadoug
Copy link
Contributor

The browser support seems quite limited and the known issues not that great:
http://caniuse.com/#feat=notifications

@felixcheung
Copy link
Member

where are we on this?

@corneadoug
Copy link
Contributor

@Stefan9149 Any plan on trying to finish this PR? Or should we just close it?

@corneadoug
Copy link
Contributor

@Stefan9149 Do we still want to go forward with this PR?

@asfgit asfgit closed this in c38a0a0 May 9, 2018
asfgit pushed a commit that referenced this pull request May 9, 2018
close #83
close #86
close #125
close #133
close #139
close #146
close #193
close #203
close #246
close #262
close #264
close #273
close #291
close #299
close #320
close #347
close #389
close #413
close #423
close #543
close #560
close #658
close #670
close #728
close #765
close #777
close #782
close #783
close #812
close #822
close #841
close #843
close #878
close #884
close #918
close #989
close #1076
close #1135
close #1187
close #1231
close #1304
close #1316
close #1361
close #1385
close #1390
close #1414
close #1422
close #1425
close #1447
close #1458
close #1466
close #1485
close #1492
close #1495
close #1497
close #1536
close #1545
close #1561
close #1577
close #1600
close #1603
close #1678
close #1695
close #1739
close #1748
close #1765
close #1767
close #1776
close #1783
close #1799
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants