AlexsLemonade / refinebio-web

Refinebio Web
https://staging.web.refine.bio
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

279 - QA: FIlter Rename Rna-seq to RNA-seq #283

Closed nozomione closed 9 months ago

nozomione commented 9 months ago

Issue Number

Closing #279

Purpose/Implementation Notes

Rename the Rna-seqname to be RNA-seq in UI.

@dvenprasad Please see the latest UI here.

Types of changes

What types of changes does your code introduce?

Functional tests

List out the functional tests you've completed to verify your changes work locally.

Checklist

vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
refinebio-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2023 8:25pm
nozomione commented 9 months ago

I'd like to see some changes before we merge this:

* move the `getFormattedTechnologyNames` to `helpers/formatTechnologyName`

* remove the nested ternary operator here and handle this by either:

  * test for default case first ie. `!['platform', 'technology'].includes(filterOption) and then short circuit for the specific special cases
  * alternatively, move this logic to a function at the top of the component that takes `filterOption` and `option`

Additionally, it looks like helpers/formatPlatformName correctly falls back to the passed in value so we can remove the || option[0] fall back.

Looking at helpers/formatPlatformName, you could further clean it up by setting the default value to an empty string and removing that first check but that change doesnt need to occur in this PR.

Thank you for the idea💡!

I made the following adjustments:

Ready for another look, thank you!