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
feat(cross-filters): add option to clear set cross filters #15500
Conversation
/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS=true |
@michael-s-molina Container image not yet published for this PR. Please try again when build is complete. |
@michael-s-molina Ephemeral environment creation failed. Please check the Actions logs for details. |
/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS=true |
@amitmiran137 Container image not yet published for this PR. Please try again when build is complete. |
@amitmiran137 Ephemeral environment creation failed. Please check the Actions logs for details. |
Codecov Report
@@ Coverage Diff @@
## master #15500 +/- ##
==========================================
- Coverage 77.20% 77.03% -0.18%
==========================================
Files 975 976 +1
Lines 50842 51247 +405
Branches 6728 6897 +169
==========================================
+ Hits 39251 39476 +225
- Misses 11376 11552 +176
- Partials 215 219 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS=true |
@villebro Ephemeral environment spinning up at http://54.188.176.58:8080. Credentials are |
@villebro Some suggestions:
|
|
||
export const FilterIndicatorText = styled.div` | ||
padding-top: ${({ theme }) => theme.gridUnit * 3}px; | ||
font-weight: ${({ theme }) => theme.typography.weights.bold}; | ||
max-width: 100%; | ||
flex-grow: 1; | ||
overflow: auto; | ||
color: ${({ theme }) => theme.colors.grayscale.light5}; | ||
`; |
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.
export const FilterIndicatorText = styled.div` | |
padding-top: ${({ theme }) => theme.gridUnit * 3}px; | |
font-weight: ${({ theme }) => theme.typography.weights.bold}; | |
max-width: 100%; | |
flex-grow: 1; | |
overflow: auto; | |
color: ${({ theme }) => theme.colors.grayscale.light5}; | |
`; | |
export const FilterIndicatorText = styled.div` | |
${({ theme }) => ` | |
padding-top: ${theme.gridUnit * 3}px; | |
font-weight: ${theme.typography.weights.bold}; | |
max-width: 100%; | |
flex-grow: 1; | |
overflow: auto; | |
color: ${theme.colors.grayscale.light5}; | |
`} | |
`; |
Good catch, I hadn't noticed that.
I agree, will make the change |
/testenv down |
Ephemeral environment shutdown and build artifacts deleted. |
/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS=true |
@villebro Ephemeral environment spinning up at http://35.166.31.72:8080. Credentials are |
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
Ephemeral environment shutdown and build artifacts deleted. |
* feat(cross-filters): add option to clear set cross filters * lint * fix indicator size, remove bolded text and rephrase text (cherry picked from commit ee2ee48)
SUMMARY
This adds the option to clear emitted cross filters by adding an
onClick
to the cross filter indicator. Also fixesAFTER
BEFORE
TESTING INSTRUCTIONS
DASHBOARD_CROSS_FILTERS
feature flagADDITIONAL INFORMATION