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

[docs] Add page feedback #22885

Merged
merged 63 commits into from
Oct 19, 2020
Merged

[docs] Add page feedback #22885

merged 63 commits into from
Oct 19, 2020

Conversation

mbrookes
Copy link
Member

@mbrookes mbrookes commented Oct 4, 2020

Page feedback

  • Add a lambda function to store page ratings and comments in DynamoDB
  • Add ratings UI to docs

Preview

UI

Feedback

image

Submit notification

image

@mbrookes mbrookes added the docs Improvements or additions to the documentation label Oct 4, 2020
@oliviertassinari

This comment has been minimized.

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 4, 2020

Details of bundle changes

Generated by 🚫 dangerJS against c5fcae6

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

This is great. Really looking forward to the feedback.

About the infra: Why is docs/rating not included in the workspaces? By including it we wouldn't need the extra install command.

About the actual UI: Since this includes quite some code, needs interactivity we and is located at the bottom we probably want to lazy load it. That way we can have a dedicated component (MarkDownDocs is quite big) and don't add that much to the initial bundle size.

eps1lon
eps1lon previously requested changes Oct 5, 2020
docs/src/modules/components/MarkdownDocs.js Outdated Show resolved Hide resolved
docs/src/modules/components/MarkdownDocs.js Outdated Show resolved Hide resolved
@mbrookes

This comment has been minimized.

@mbrookes mbrookes marked this pull request as draft October 5, 2020 16:42
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

A couple of thoughts:

  • If we include the version, we could also include the language too in the feedback record.
  • I have a few doubts about DynamoDB as the best database for our problem. DynamoDB does not provide aggregation functions. So if we want to compute the global mean for all the pages, compare the rating for each version, for each language, for different quarters, etc. We won't be able to do it easily. It seems that we would need to go into the ETL land, load the data in a Redshift, or in Athena, etc. Not as simple as running a SQL AVG() aggregation function.
  • I wonder about the precision of: rating = (average * count + request.body.rating) / (count + 1); once you get >1,000 feedback.

@mbrookes mbrookes requested a review from eps1lon October 15, 2020 01:28
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 18, 2020
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Oct 19, 2020
@mbrookes mbrookes force-pushed the docs-ratings branch 3 times, most recently from 2ec33b4 to 1df3b19 Compare October 19, 2020 20:29
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 19, 2020
@mbrookes mbrookes merged commit 8d00f6d into mui:next Oct 19, 2020
@mbrookes mbrookes deleted the docs-ratings branch October 19, 2020 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants