rusackas commented on a change in pull request #10446:
URL:
https://github.com/apache/incubator-superset/pull/10446#discussion_r461307722
##########
File path: superset-frontend/src/components/ListView/Filters.tsx
##########
@@ -48,9 +48,12 @@ interface SelectFilterProps extends BaseFilter {
paginate?: boolean;
}
+const FONT_SIZE = 12;
+
const FilterContainer = styled.div`
display: inline-flex;
margin-right: 2em;
+ font-size: ${FONT_SIZE}px;
Review comment:
```suggestion
font-size: ${({ theme }) => theme.typography.sizes.s};
```
##########
File path: superset-frontend/src/components/ListView/Filters.tsx
##########
@@ -63,6 +66,7 @@ const filterSelectTheme: PartialThemeConfig = {
spacing: {
baseUnit: 2,
minWidth: '5em',
+ fontSize: FONT_SIZE,
Review comment:
```suggestion
fontSize: ${({ theme }) => theme.typography.sizes.s},
```
##########
File path: superset-frontend/src/components/ListView/Filters.tsx
##########
@@ -48,9 +48,12 @@ interface SelectFilterProps extends BaseFilter {
paginate?: boolean;
}
+const FONT_SIZE = 12;
Review comment:
```suggestion
```
##########
File path: superset-frontend/src/components/ListView/Filters.tsx
##########
@@ -48,9 +48,12 @@ interface SelectFilterProps extends BaseFilter {
paginate?: boolean;
}
+const FONT_SIZE = 12;
+
const FilterContainer = styled.div`
display: inline-flex;
margin-right: 2em;
+ font-size: ${FONT_SIZE}px;
Review comment:
```suggestion
font-size: ${({ theme }) => theme.typography.sizes.s}px;
```
##########
File path: superset-frontend/src/components/ListView/Filters.tsx
##########
@@ -63,6 +66,7 @@ const filterSelectTheme: PartialThemeConfig = {
spacing: {
baseUnit: 2,
minWidth: '5em',
+ fontSize: FONT_SIZE,
Review comment:
```suggestion
fontSize: ${({ theme }) => theme.typography.sizes.s}px,
```
##########
File path: superset-frontend/src/components/ListView/Filters.tsx
##########
@@ -63,6 +66,7 @@ const filterSelectTheme: PartialThemeConfig = {
spacing: {
baseUnit: 2,
minWidth: '5em',
+ fontSize: FONT_SIZE,
Review comment:
Nope! It should be available via the ThemeProvider way up at the top of
the app.
##########
File path: superset-frontend/src/components/ListView/Filters.tsx
##########
@@ -63,6 +66,7 @@ const filterSelectTheme: PartialThemeConfig = {
spacing: {
baseUnit: 2,
minWidth: '5em',
+ fontSize: FONT_SIZE,
Review comment:
In fact, it's _better_ to use the theme through the provider than
importing it from the `superset-ui` package. Then we can do fun stuff with it,
e.g. `<ThemeProvider theme={...supersetTheme, ...darkMode,
...myCompanyBrandColors}>`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
