Skip to content

web: analytics nudge in sidebar (placeholder, styling TBD) [ch2531]#1622

Merged
maiamcc merged 2 commits into
masterfrom
maiamcc/nudge-in-web
May 14, 2019
Merged

web: analytics nudge in sidebar (placeholder, styling TBD) [ch2531]#1622
maiamcc merged 2 commits into
masterfrom
maiamcc/nudge-in-web

Conversation

@maiamcc

@maiamcc maiamcc commented May 13, 2019

Copy link
Copy Markdown
Contributor

Hello @yuindustries, @jazzdan,

Styling TBD (look there's even a ticket for it) but this PR displays the analytics nudge in the sidebar. If it's a long list of entries, we show the nudge 3rd in the list to avoid it getting lost in the shuffle. Awkward left-border to make it visible even when the pane is collapsed. Enjoy this hot mess of CSS! (don't worry, this is all gated behind an env variable.)

Screen Shot 2019-05-13 at 5 02 15 PM

@maiamcc
maiamcc requested review from hyu and jazzdan May 13, 2019 21:28
@maiamcc maiamcc changed the title web: analytics nudge in sidebar [ch2531] web: analytics nudge in sidebar (placeholder, styling TBD) [ch2531] May 13, 2019
@maiamcc

maiamcc commented May 13, 2019

Copy link
Copy Markdown
Contributor Author

And here it is collapsed:

Screen Shot 2019-05-13 at 5 31 19 PM

@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.

Should this be a popover rather than a part of the sidebar? Overall the code LGTM though.

Comment thread web/src/AnalyticsNudge.tsx Outdated
<div className="AnalyticsNudge" key="AnalyticsNudge">
<div>
Congrats on your first Tilt resource 🎉 Help us help you: may we
collect anonymized data on your usage. (Read more{" "}

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.

Should this end in a question mark?

may we collect anonymized data on your usage?

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.

...GEEZ DAN THAT'S WHAT I SAID THE FIRST TIME WEREN'T YOU LISTENING 👀

@maiamcc

maiamcc commented May 13, 2019

Copy link
Copy Markdown
Contributor Author

I'm happy to have this be a popover! Idk how to do that nonsense am happy to learn :)

am likely gonna not implement that in this PR, cuz again, trying to get the backend wired up, but will maybe do for future.

@maiamcc
maiamcc merged commit 516e2c0 into master May 14, 2019
@maiamcc
maiamcc deleted the maiamcc/nudge-in-web branch May 14, 2019 15:05
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.

2 participants