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: Adding report bug #16065
fix: Adding report bug #16065
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16065 +/- ##
==========================================
- Coverage 76.80% 76.80% -0.01%
==========================================
Files 995 995
Lines 52866 52868 +2
Branches 6713 6714 +1
==========================================
Hits 40605 40605
- Misses 12035 12037 +2
Partials 226 226
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
53b9271
to
ceaf0a0
Compare
@@ -60,14 +60,15 @@ export default function HeaderReportActionsDropDown({ | |||
}; | |||
|
|||
const menu = () => ( | |||
<Menu selectable={false}> | |||
<Menu selectable={false} css={{ width: '200px' }}> |
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.
Shouldn't this be style
instead of css
?
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.
emotion uses css, and I believe we prefer that over style which hits the dom directly.
<Menu.Item> | ||
{t('Email reports active')} | ||
<Switch | ||
data-test="toggle-active" | ||
checked={report?.active} | ||
onClick={(checked: boolean) => toggleActiveKey(report, checked)} | ||
size="small" | ||
css={{ marginLeft: '8px' }} |
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.
Same here?
Also, should we use grid units here?
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.
+1 on the grid units
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.
good point! Added, ty.
@betodealmeida I'm going to merge, but feel free to comment if you think something wasn't addressed (looks like everything was). |
🏷 2021.31 |
* report add fix * added theme (cherry picked from commit 4359650)
* report add fix * added theme (cherry picked from commit 4359650)
* report add fix * added theme (cherry picked from commit 4359650)
* report add fix * added theme
* report add fix * added theme (cherry picked from commit 4359650)
* report add fix * added theme
* report add fix * added theme (cherry picked from commit 4359650)
* report add fix * added theme (cherry picked from commit ccfd16d)
SUMMARY
The reducer for the add reports feature was being sent the wrong information, this fixes it. It also adds margin left to the toggle active button.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Screen.Recording.2021-08-04.at.11.37.37.AM.mov
TESTING INSTRUCTIONS
Go to dashboards with ALERT_REPORTS set to true, and a role that has admin. Add a report.
ADDITIONAL INFORMATION