refactor(app): clear styled-components out of LabwareDetails and HistoricalRunProtocol#21166
refactor(app): clear styled-components out of LabwareDetails and HistoricalRunProtocol#21166nataliet87 merged 5 commits intoedgefrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## edge #21166 +/- ##
==========================================
- Coverage 57.20% 57.20% -0.01%
==========================================
Files 3987 3987
Lines 326741 326735 -6
Branches 46475 46473 -2
==========================================
- Hits 186910 186898 -12
- Misses 139612 139618 +6
Partials 219 219
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
SyntaxColoring
left a comment
There was a problem hiding this comment.
This looks great, thank you!
Some GitHub tips:
-
GitHub auto-created the PR title
App refactor styled componentsfrom your branch nameapp_refactor-styled-components. When you get the chance, edit it to the format likerefactor(app): Blah blah. That'll make it easier to scan in the PR list. And it'll help generate the correct commit name later, when this PR is approved and you merge it intoedge. -
Instead of (or in addition to) @-mentioning people in a comment, you can name them as reviewers in the "Reviewers" section in the top-right. That way, the PR will show up on their side as "review requested" instead of just "mentioned," which can help with filtering and notifications.
-
Just a reminder, before un-drafting this, try to fill out the PR template to the best of your ability. And since this is your first PR, also read through our PR etiquette guidelines, if you haven't already. (I think you'll be good, I'm just making sure someone's introduced you to those at least once.)
Feel free to run stuff by us if you're not sure.
I know it can feel like a lot of formality the first time, sorry. It'll feel more casual as you keep going.
Below are some comments about the code.
f7df3cc to
2b202ae
Compare
TamarZanzouri
left a comment
There was a problem hiding this comment.
code change LGTM once CI is green! if you can add screemshots if would be helpful
SyntaxColoring
left a comment
There was a problem hiding this comment.
Great job!
Some final reminders when you click the merge button:
-
Make sure the first line (the summary) matches our commit message conventions. It should look like
refactor(app): clear styled-components out of LabwareDetails and HistoricalRunProtocol (#21166), matching the PR title. -
Replace the extended description with something concise. What to include is largely your discretion, but definitely make sure that you delete any junk left over from the PR template, like "Describe your PR at a high level...."
One reasonable extended description would be just a copy-paste of your Overview section. i.e.
This change updates the syntax from styled-components to modules.css in App LabwareDetails and HistoricalRunProtocol files.
From https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#after-receiving-pr-approval
|
Oh shoot, I'm not sure what happened to the screenshots I'd thought I attached @TamarZanzouri Trying again here:
- New:
- New:
- New:
- New:
|










Overview
Describe your PR at a high level. State acceptance criteria and how this PR fits into other work. Link issues, PRs, and other relevant resources.
This change updates the syntax from
styled-componentstomodules.cssin AppLabwareDetailsandHistoricalRunProtocolfiles.Test Plan and Hands on Testing
Ran make -C app dev and verify that everything looks good.
Changelog
styled-componentsusage removed fromLabwareDetails/index.tsxandHistoricalRunProtocol.tsxfiles withmodule.cssfiles added holding component definitions.Review requests
Risk assessment