-
Notifications
You must be signed in to change notification settings - Fork 178
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
fix(app): always enable BackToTop button #14103
fix(app): always enable BackToTop button #14103
Conversation
Remove checks for disabling BackToTop button at the last setup step and remove hover tooltips for disabled reason. The user should always be enabled to click BackToTop in protocol setup even if proceeding to run is disabled.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## chore_release-7.1.0 #14103 +/- ##
=======================================================
- Coverage 70.44% 70.42% -0.02%
=======================================================
Files 1636 2512 +876
Lines 54474 71232 +16758
Branches 4008 8975 +4967
=======================================================
+ Hits 38373 50167 +11794
- Misses 15407 18872 +3465
- Partials 694 2193 +1499
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, BackToTopButton
test suite can be cleaned up a little bit. I think lines 28-37 can be deleted since we no longer need those mocks.
app/src/organisms/Devices/ProtocolRun/__tests__/BackToTopButton.test.tsx
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/ProtocolRun/__tests__/BackToTopButton.test.tsx
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/ProtocolRun/__tests__/BackToTopButton.test.tsx
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/ProtocolRun/__tests__/BackToTopButton.test.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! thanks for cleaning up the test 🦕
closes RQA-1954
Overview
Remove checks for disabling
BackToTop
button at the last setup step and remove hover tooltips for disabled reason. The user should always be enabled to clickBackToTop
inProtocolRun
even if proceeding to run is disabled.Note: I set the disabled prop for the button to always
false
because the only time component is rendered is withinProtocolRun
. If we utilize the button elsewhere, we can add a prop for disabling.Test Plan
BackToTop
button renders after the last setup step and is enabled.Changelog
BackToTop
inside componentRisk assessment
low