ooni / ooni/explorer

Refactor SummaryText

Open
#156 3 comments 0 reactions 1 assignee View on GitHub

Nobody has claimed this yet.

chore
Dominant language
JavaScript
Stars
92
Forks
45
PR merge metrics
No merged PRs in 30d

Description

As @sarathms wrote in: https://github.com/ooni/explorer/pull/146/#discussion_r282578340

Turns out it is a bug, yet works as expected. It was certainly not intended this way. react-intl fails to find the key and prints the key itself as the message.

I have modified SummaryText to accept either a function, string, or any in the content prop and render them accordingly. The problem with string type is that it cannot distinguish between the react-intl key and an actual string to be rendered (in this case already formatted).

One fix is sending the key as an object to the content prop e.g content={{ key: 'Measurement.Test.SummaryText.Reason'}} with additional values if needed. And handle it the way typeof content === 'string' is handled right now. And render strings as is.

I feel the measurement details section in general and the SummaryText component in particular have gone through many iterations and I haven't been able to apply the patterns consistently across components/measurement/nettests/*.js files. I wonder if I should refactor it based on what we know now about what we want.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.