- Notifications
You must be signed in to change notification settings - Fork1.1k
feat(site): add template insights page#8722
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
mtojek commentedJul 25, 2023
@BrunoQuaresma could you please post some screenshots? |
BrunoQuaresma commentedJul 25, 2023
@mtojek I posted a video! |
mtojek left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good to me 👍 I played a bit with this, and seems to work correctly.
Could you please write in the PR description what are the next steps? I assume that this is the MVP.
| // not be included here and will be essentially hidden. | ||
| varExperimentsAll=Experiments{} | ||
| varExperimentsAll=Experiments{ | ||
| ExperimentTemplateInsightsPage, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is it intended to release as experimental?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, I talked with@bpmct about this
| @@ -1,37 +0,0 @@ | |||
| import{render}from"testHelpers/renderHelpers" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is this deletion intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeap, it is just a render test which should be on the storybook.
| } | ||
| } | ||
| functionsevenDaysAgo(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nit: weekAgo :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is a good one. I don't think a "week ago" is a good name for this because you are at Wed and a week ago would be the previous week from Mon to Fri so 7 days ago is different and for this case would be more accurate
| "@types/connect" "*" | ||
| "@types/node" "*" | ||
| "@types/chroma-js@2.4.0": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is this change intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
yes, it is!
Uh oh!
There was an error while loading.Please reload this page.
Close#8689
Demo:
Screen.Recording.2023-07-25.at.13.49.09.mov
Next steps: