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

proposal: x/tools/cmd/present: Support presenter notes on the Go present tool #14654

Closed
audreylim opened this issue Mar 5, 2016 · 15 comments
Closed

Comments

@audreylim
Copy link
Contributor

@audreylim audreylim commented Mar 5, 2016

I propose adding support for presenter notes in the Go present tool.

Motivation: Go present is great. I used Go present at the recent GopherCon India and wished there was support for presenter notes. Having a presenter tool, with Go present, can make presentations a lot smoother and calm the presenter's nerves. Presenters don't have to use alternatives with presenter options if they want to use Go present. Best of both worlds.

Demo: http://virtivia.com:27080/1vs4fiwjm924q.mov

An implementation: https://github.com/audreylim/go-presenter#readme

If this proposal is accepted, I can submit a CL with the changes I've done for further review.

EDIT: here are the changes

@kostya-sh
Copy link
Contributor

@kostya-sh kostya-sh commented Mar 7, 2016

Related issue: #12355

@ianlancetaylor ianlancetaylor added this to the Proposal milestone Mar 7, 2016
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 7, 2016

CC @adg

@audreylim
Copy link
Contributor Author

@audreylim audreylim commented Mar 8, 2016

Summary of changes (latest): audreylim/go-presenter@6d2d6fe...9b35c29

Technical implementation:

  • Presenter uses localStorage event listeners to sync both windows - the main browser window and the pop up window, which will display the slides and presenter notes
  • Processing is done entirely in the front end (javascript), no websockets involved
  • The main browser window listens for keypress 'P', which will open the Presenter window. However, 'P' only works when Presenter mode is enabled, by running present -p instead of present
  • Presenter features will run only if Presenter mode is enabled, so none of its features (eg. local storage, event listeners) will interfere with the original present if that is not the intent
  • Instruction for pressing 'P' appears in the terminal logs when present -p is run - for informing the speaker
  • The slides are loaded onto the Presenter window using an iframe - this retains the layout of the Go present slides, and allows dual syncing triggered by localStorage events, which ensures reliability and speed of the syncing
  • Every time the slides are updated on either the Presenter window or main browser window, the latest slide index is stored in localStorage - the event listener catches this and triggers the update in the other window
  • The notes are updated on the Presenter window by listening and retrieving the updated slide number stored in localStorage
  • Notes are parsed/identified in the backend by prefixing a paragraph with "& ". Multiple paragraphs are supported

Notes/Concerns:

  • Running .play actions by clicking on Run does not sync in the other window (see example)
  • Currently does not display timer, but that can be added (also indicate current slide number)
  • The focus was to get this working, so it currently supports only Chrome. Multiple browser support can be investigated/enabled
  • This implementation is meant to help speakers primarily, so speaker notes will not be directly available on sites like go-talks.appspot.com for readers/audiences. It works only if run locally as present -p can be enabled
  • "& " is the current syntax for notes, but that can be changed. Maybe ("// ") or something comment/notes like
  • The Presenter window currently displays a sneak preview (one-tenth) of the previous and next slide. If the preference is to view the entire slide, the window width can be changed

Please let me know what you think.

@adg
Copy link
Contributor

@adg adg commented Mar 9, 2016

I think the general approach looks fine. We'll need to resolve the issue with run not being synchronized. The state of the presenter window and the main window should be completely synchronized.

Hey @robpike (who has some ownership over the present file format) do you think that & is a fine start-of-line symbol for presenter notes?

@robpike
Copy link
Contributor

@robpike robpike commented Mar 9, 2016

I think it doesn't matter much but I might choose > or even -> .

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Mar 10, 2016

I think it doesn't matter much but I might choose > or even -> .

I don't mean to bikeshed, but I would mention the following about > as the prefix:

> is often used as a prefix for doing a quote or blockquote. It's what it looks like:

> some quote goes here

some reply goes here

Which is also valid Markdown that gets rendered as:

some quote goes here

some reply goes here

-> prefix does not have this problem as far as I know.

However, I looked at some existing talks, and did not find any usages of this markdown idiom there. The one quote I found was actually written as a quote rather than a blockquote, so maybe > is not bad (even if it conflicts with markdown).

@adg
Copy link
Contributor

@adg adg commented Mar 10, 2016

-> seems unnecessarily hard to type, imo.

Another options is :, which is (too?) similar to the comment character ;.

Slide content

& Speaker note
Slide content

> Speaker note
Slide content

: Speaker note

I'm actually kinda partial to :. More so than & at least. Using colon has the property that it doesn't already mean something in markdown, and it is very unlikely to have been used in slides to mean anything. (It's conceivable but unlikely someone might have started a line with &, though.)

I too apologize for bike shedding.

@audreylim
Copy link
Contributor Author

@audreylim audreylim commented Mar 10, 2016

I liked > but as @shurcooL pointed out, it conflicts with Markdown convention.

The next best choice seems to be :. It's more correlated to "speaking" than &.

Eg.

Person A says: hello
Person B says: hi

My vote goes to :.

@adg
Copy link
Contributor

@adg adg commented Mar 10, 2016

Okay, let's go with : assuming there are no fierce objections.

Implementation-wise, I suggest going in this order:

  • Parse notes and record them in the Section struct
  • Add the command-line flag to enable presenter notes
  • Add the notes to the slide template
  • Display the notes, synchronize browser windows
  • Docs

I also suggest, since notes seem to be the focus here, that the flag be -notes rather than -p.

@adg
Copy link
Contributor

@adg adg commented Mar 10, 2016

I would consider issue #12355 closed once this is resolved. The approach described there is overkill, I think.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Mar 10, 2016

I like :. More than &.

After seeing @audreylim's analogy to "Person A says: hello", that makes it easy to remember too.

@audreylim
Copy link
Contributor Author

@audreylim audreylim commented Mar 10, 2016

@adg Noted your suggestions.

Minor: With the -notes flag instead of -p, any preference for the keypress? It's currently P (for Presenter) to open a window. With notes, might be more consistent to use keypress N

@adg
Copy link
Contributor

@adg adg commented Mar 10, 2016

N also sounds good. 👍

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 4, 2016

CL https://golang.org/cl/21485 mentions this issue.

gopherbot pushed a commit to golang/tools that referenced this issue Apr 7, 2016
This is the first of a series of changes that add support for
speaker notes to the Go present tool. This is done by displaying
slides with speaker notes on a second window, and synchronizing
both windows.

Updates golang/go#14654

Change-Id: Ic7b158d1e40f9e7e58d01791c88909f5619ce87f
Reviewed-on: https://go-review.googlesource.com/21485
Reviewed-by: Andrew Gerrand <adg@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 24, 2016

CL https://golang.org/cl/21489 mentions this issue.

jjneely pushed a commit to jjneely/present that referenced this issue Nov 4, 2016
Fixes golang/go#14654

Fixes golang/go#12355

Change-Id: I0c05db624170f7bef5883192b47b618ca343a830
Reviewed-on: https://go-review.googlesource.com/21489
Reviewed-by: Andrew Gerrand <adg@golang.org>
@golang golang locked and limited conversation to collaborators Jun 1, 2017
@dmitshur dmitshur added the Proposal label Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.