Skip to content

web: design snapshot modal#2160

Merged
hyu merged 5 commits into
masterfrom
hyu/ch3358/snapshot-modal
Sep 6, 2019
Merged

web: design snapshot modal#2160
hyu merged 5 commits into
masterfrom
hyu/ch3358/snapshot-modal

Conversation

@hyu

@hyu hyu commented Sep 6, 2019

Copy link
Copy Markdown
Contributor

Made a pass at the copy, too.

Token Not Registered

2019-09-05 - Snapshot modal - sign up

Token Registered

2019-09-05 - Snapshot modal

@hyu
hyu requested review from landism and maiamcc September 6, 2019 00:09

@jazzdan jazzdan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@landism landism left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice!

href="http://test.com"
target="_blank"
<p>
Let anyone explore your current Tilt session with an interactive snapshot.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be good to emphasize:

  1. read-only
  2. not live

to provide assurance that they're only sharing a limited thing (don't need to worry about anyone else modifying your stuff, don't need to worry about keeping your Tilt in the state you want the user to see, and don't need to worry about whether any secrets logged in the future are now available via the snapshot). Maybe some of that could be inferred from "snapshot", but more explicitness seems like it'd yield more feelings of safety.

stab: "Let anyone explore a read-only copy of the current state of your Tilt session with an interactive snapshot"

The "anyone" also kinda gives me pause, like "if I share this, can the whole WORLD see it!?"
We could say "anyone with the link", a la google docs. We do at least provide that level of privacy, and are not currently communicating it anywhere. That might be too wordy.

Copy isn't the point of this PR though. If you don't wanna mess with this, LMK and I'll file a ticket for us to deal with it later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is great! This is precisely what I was thinking through as well, and trying to get a succinct version that sounds accessible and breezy is...tricky. I won't tinker further in this PR but I think we can have a story for "final round of copyediting" to address these branding and technical accuracy issues.

</div>
<section>
<p>
Register on TiltCloud to share under your name, view, and delete

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh man
potential can o' worms time
have we settled on "TiltCloud" vs "Tilt Cloud"? I've been using "Tilt Cloud"

(not important for this PR)

<section>
<p>
Register on TiltCloud to share under your name, view, and delete
snapshots.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

*your snapshots

(or maybe "to share under your name and manage your snapshots"...the "view" feels weird to me, since you can view them without registering)

return <div>Sharing snapshots as {this.props.tiltCloudUsername}.</div>
return (
<p>
View and delete snapshots on{" "}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

as above

@maiamcc maiamcc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a few pedantic copy suggestions, feel free to ignore. this looks great!

<input
className="ShareSnapshotModal-tiltCloudButton"
type="submit"
value="Sign Up"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this copy isn't quiiiite right. needs to be like "sign up / link account" or something (cuz they may already be signed up for tilt cloud, and it's just associated with this Tilt instance yet)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree, the "Sign Up" is misleading. I will consider a better way to express that you're linking your Tilt instance.

<div className="ShareSnapshotModal-pane">
<p>
Let anyone explore your current Tilt session with an interactive
snapshot.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i don't feel strongly but maybe this gets more clearly at the fact that the state, while interactive, is static:
"Let anyone explore your dev environment with an interactive snapshot of your current Tilt state."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was playing with "session" vs "state" and you just nudged me into thinking the latter may be better.

</div>
<section>
<p>
Register on TiltCloud to share under your name, view, and delete

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Connect your Tilt install with a TiltCloud account...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Duly noted...I am ingesting this useful feedback and mulling on it. I think we should just have one last copy round in a separate PR.

@hyu
hyu merged commit 84230a5 into master Sep 6, 2019
@nicks
nicks deleted the hyu/ch3358/snapshot-modal branch February 3, 2022 16:50
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.

4 participants