ethereum / ethereum-org-website

Ethereum.org is a primary online resource for the Ethereum community.
https://ethereum.org/
MIT License
4.9k stars 4.67k forks source link

Create story for Quiz components #13048

Open corwintines opened 1 month ago

corwintines commented 1 month ago

Create stories that test the following quiz components:

TylerAPfledderer commented 2 weeks ago

Commenting for assignment, to have this completed in conjunction with #11729

TylerAPfledderer commented 2 weeks ago

@corwintines question here that would also be a question for @pettinarip:

With QuizWidget, it would makes sense to have each part as is it's own story to ensure proper visual testing of each part without creating interactive snapshots in addition to the two existing interactive tests that check for the results.

Take the QuizSummary component as the primary issue I see. The logic provided to it is coupled in the comment through a useContext hook. This adds complexity to making a story for it because I would have to build out the matching provider and passing in all the required values that would not be used in QuizSummary.

Could I pull the logic out into a wrapper, so then I have a component base to export to Storybook that only requires passing the needed props and rendering the needed styles?


/**
  * Renders the styling and the content for the summary section 
  * of the `QuizWidget`
  *
  * NOTE: This is not directly used for production, but for Storybook visual testing.
  * This is because all the content passed to it comes from `useQuizWidgetContext()`.
  * Please use `QuizSummary` default export for production use only!
  */
export const StyledQuizSummary = (internalProps) => {
  // Render the component styles and the content from props
}

const QuizSummary = () => {
  // Get the logic then send to the styled compoent

  return <StyledQuizSummary {...logicProps} />
}

export default QuizSummary

So production uses QuizSummary and Storybook uses StyledQuizSummary.

pettinarip commented 2 weeks ago

@TylerAPfledderer Im ok with that approach.

However, looking at the code I see that the direct parent of QuizSummary is the QuizWidget and the latter knows all the information needed by QuizSummary. So, wouldn't it be easier to just remove the hook and pass the data as props?

<QuizWidgetProvider value={{...}}>
  <QuizSummary
    numberOfCorrectAnswers={numberOfCorrectAnswers}
    questions={quizData.questions}
    ratioCorrect={ratioCorrect}
    isPassingScore={isPassingScore}
  />
</QuizWidgetProvider>

With that we would avoid the complexity of breaking it out.

TylerAPfledderer commented 2 weeks ago

@pettinarip

Fair point! That makes more sense.

Further reviewing that, using a provider might not be so practical here!. Better to just pass the props than send through context with all the components.

I can look at passing the logic to all of them instead of context so they are more pure.

corwintines commented 14 hours ago

Makes sense to me here!