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

x/tools/cmd/present: notes mode causes different presentations in separate tabs/windows to advance in step #24688

Closed
myitcv opened this issue Apr 4, 2018 · 4 comments

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Apr 4, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10.1 linux/amd64
present at commit ac136b6c2db7c4d43955e4bc7174db36dc0539c0

Does this issue reproduce with the latest release?

Yes, per above

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build270412448=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Open the following two presentations in Chrome in two separate tabs in the same window (could be any two presentations, these are just current examples):

https://talks.godoc.org/github.com/stmuk/go-unicode-examples/unicode.slide
https://talks.godoc.org/github.com/mvdan/talks/2018/benchstat.slide

Observe that both start on slide number 1.

Now click to advance the Unicode presentation forward by one slide.

What did you expect to see?

Only the Unicode presentation should advance.

What did you see instead?

If you switch to the tab with the Benchstat presentation you will notice that has also advanced to slide number 2.

This appears to happen because:

Seemingly this behaviour is necessary for notes mode where the notes window and the main deck should advance pari passu (and talks.godoc.org operates in notes mode)

But where there are two different presentations (this issue came up at the London Gophers meetup) then this is less than desirable.

Could the path (window.location.pathname) be used to distinguish the keys?

As a postscript, if you try to advance the Benchstat presentation beyond slide 13 you will likely find that Chrome's CPU usage jumps because it gets into something of a loop. This appears to be because the Unicode presentation does not have 14 slides and so some loop is triggered.

@gopherbot
Copy link

@gopherbot gopherbot commented May 24, 2019

Change https://golang.org/cl/178659 mentions this issue: cmd/present: use unique key for destSlide in local store

@danicat
Copy link
Contributor

@danicat danicat commented May 24, 2019

Just submitted a fix for this. I was expecting to write some Go code, ended up writing javascript. 😂

I'm a bit concerned about the lack of automated tests in this project, but I don't have enough JS knowledge to tackle this. Nevertheless, tested locally and it seems to do the job.

@myitcv
Copy link
Member Author

@myitcv myitcv commented May 26, 2019

I'm a bit concerned about the lack of automated tests in this project

It was the lack of automated tests that alarmed you? It was the very presence of JavaScript that set alarm bells ringing for me 😄

@danicat
Copy link
Contributor

@danicat danicat commented May 28, 2019

Totally with you on that @myitcv ! I was just afraid to address the elephant in the room I guess... lol

I've just submitted a new patch with style fixes. I may try to add some test coverage to it, but I'm not sure if I'll have the time to do it anytime soon.

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
4 participants
You can’t perform that action at this time.