You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "geido (via GitHub)" <gi...@apache.org> on 2023/02/21 15:18:27 UTC
[GitHub] [superset] geido opened a new pull request, #23138: feat: Cross Filters in FilterBar
geido opened a new pull request, #23138:
URL: https://github.com/apache/superset/pull/23138
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1442022730
@kgabryje Ephemeral environment spinning up at http://34.215.230.191:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1441751254
@kasiazjc about the "No filters" empty state that is referring to the native filters and it is mentioning the button to add new filters. Should we change the copy a bit to make sure it is referring to the native filters and not cross filters?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on a diff in pull request #23138: feat: Cross Filters in FilterBar
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #23138:
URL: https://github.com/apache/superset/pull/23138#discussion_r1114130386
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarCrossFilters/CrossFilter.tsx:
##########
@@ -0,0 +1,229 @@
+/**
Review Comment:
That's a really minor nitpick but... since we're already in `FilterBar` directory, maybe we should name the subdirectory `CrossFilters` instead of `FilterBarCrossFilters`?
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarCrossFilters/CrossFilter.tsx:
##########
@@ -0,0 +1,229 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { useCallback } from 'react';
+import { styled, t, css, useTheme } from '@superset-ui/core';
+import { CrossFilterIndicator } from 'src/dashboard/components/nativeFilters/selectors';
+import Icons from 'src/components/Icons';
+import { useDispatch } from 'react-redux';
+import { setFocusedNativeFilter } from 'src/dashboard/actions/nativeFilters';
+import { Tag } from 'src/components';
+import { Tooltip } from 'src/components/Tooltip';
+import useCSSTextTruncation from 'src/hooks/useTruncation/useCSSTextTruncation';
+import { FilterBarOrientation } from 'src/dashboard/types';
+import { updateDataMask } from 'src/dataMask/actions';
+
+const StyledCrossFilterTitle = styled.div`
+ ${({ theme }) => `
+ display: flex;
+ font-size: ${theme.typography.sizes.s}px;
+ color: ${theme.colors.grayscale.base};
+ vertical-align: middle;
+ cursor: pointer;
+ align-items: center;
+ `}
+`;
+const StyledIconSearch = styled(Icons.SearchOutlined)`
+ ${({ theme }) => `
+ color: ${theme.colors.grayscale.light1};
+ margin-left: ${theme.gridUnit * 2}px;
+ &:hover {
+ color: ${theme.colors.grayscale.base};
+ }
+ `}
+`;
+
+const ellipsisCss = css`
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: inline-block;
+ vertical-align: middle;
+`;
+
+const StyledCrossFilterValue = styled('b')`
+ ${({ theme }) => `
+ max-width: ${theme.gridUnit * 25}px;
+ `}
+ ${ellipsisCss}
+`;
+
+const StyledCrossFilterColumn = styled('span')`
+ ${({ theme }) => `
+ max-width: ${theme.gridUnit * 25}px;
+ padding-right: ${theme.gridUnit}px;
+ `}
+ ${ellipsisCss}
+`;
+
+const CrossFilterTag = (props: {
+ filter: CrossFilterIndicator;
+ orientation: FilterBarOrientation;
+ removeCrossFilter: (filterId: number) => void;
+}) => {
+ const { filter, orientation, removeCrossFilter } = props;
+ const theme = useTheme();
+ const [columnRef, columnIsTruncated] =
+ useCSSTextTruncation<HTMLSpanElement>();
+ const [valueRef, valueIsTruncated] = useCSSTextTruncation<HTMLSpanElement>();
+
+ return (
+ <Tag
+ css={css`
+ ${orientation === FilterBarOrientation.VERTICAL
+ ? `
+ margin-top: ${theme.gridUnit * 2}px;
+ `
+ : `
+ margin-left: ${theme.gridUnit * 2}px;
+ `}
+ `}
+ closable
+ onClose={() => removeCrossFilter(filter.emitterId)}
+ >
+ <Tooltip title={columnIsTruncated ? filter.column : null}>
+ <StyledCrossFilterColumn ref={columnRef}>
+ {filter.column}
+ </StyledCrossFilterColumn>
+ </Tooltip>
+ <Tooltip title={valueIsTruncated ? filter.value : null}>
+ <StyledCrossFilterValue ref={valueRef}>
+ {filter.value}
+ </StyledCrossFilterValue>
+ </Tooltip>
+ </Tag>
+ );
+};
+
+const CrossFilterChartTitle = (props: {
+ title: string;
+ orientation: FilterBarOrientation;
+}) => {
+ const { title, orientation } = props;
+ const [titleRef, titleIsTruncated] = useCSSTextTruncation<HTMLSpanElement>();
+ const theme = useTheme();
+ return (
+ <Tooltip title={titleIsTruncated ? title : null}>
+ <span
+ css={css`
+ max-width: ${orientation === FilterBarOrientation.VERTICAL
+ ? `${theme.gridUnit * 60}px`
+ : `${theme.gridUnit * 15}px`};
+ line-height: 20px;
Review Comment:
should we use a relative line height? Like 1.4 or something
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx:
##########
@@ -126,37 +170,53 @@ const FilterControls: FC<FilterControlsProps> = ({
</>
);
- const items = useMemo(
- () =>
- filtersInScope.map((filter, index) => ({
- id: filter.id,
- element: (
- <div
- className="filter-item-wrapper"
- css={css`
- flex-shrink: 0;
- `}
- >
- {renderer(filter, index)}
- </div>
- ),
- })),
- [filtersInScope, renderer],
- );
+ const items = useMemo(() => {
+ const crossFilters = selectedCrossFilters.map(c => ({
+ // a combination of filter name and chart id to account
+ // for multiple cross filters from the same chart in the future
+ id: `${c.name}${c.emitterId}`,
+ element: rendererCrossFilter(
+ c,
+ FilterBarOrientation.HORIZONTAL,
+ selectedCrossFilters.at(-1),
+ ),
+ }));
+ const nativeFiltersInScope = filtersInScope.map((filter, index) => ({
+ id: filter.id,
+ element: (
+ <div
+ className="filter-item-wrapper"
+ css={css`
+ flex-shrink: 0;
+ `}
+ >
+ {renderer(filter, index)}
+ </div>
+ ),
+ }));
+ return [...crossFilters, ...nativeFiltersInScope];
+ }, [filtersInScope, renderer, rendererCrossFilter, selectedCrossFilters]);
const overflowedFiltersInScope = useMemo(
() => filtersInScope.filter(({ id }) => overflowedIds?.includes(id)),
[filtersInScope, overflowedIds],
);
- const activeOverflowedFiltersInScope = useMemo(
+ const overflowedCrossFilters = useMemo(
() =>
- overflowedFiltersInScope.filter(filter =>
- isNativeFilterWithDataMask(filter),
+ selectedCrossFilters.filter(({ emitterId, name }) =>
+ overflowedIds?.includes(`${name}${emitterId}`),
),
- [overflowedFiltersInScope],
+ [overflowedIds, selectedCrossFilters],
);
+ const activeOverflowedFiltersInScope = useMemo(() => {
+ const activerOverflowedFilters = overflowedFiltersInScope.filter(filter =>
Review Comment:
typo π `activerOverflowedFilters` -> `activeOverflowedFilters`
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarCrossFilters/CrossFilter.tsx:
##########
@@ -0,0 +1,229 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { useCallback } from 'react';
+import { styled, t, css, useTheme } from '@superset-ui/core';
+import { CrossFilterIndicator } from 'src/dashboard/components/nativeFilters/selectors';
+import Icons from 'src/components/Icons';
+import { useDispatch } from 'react-redux';
+import { setFocusedNativeFilter } from 'src/dashboard/actions/nativeFilters';
+import { Tag } from 'src/components';
+import { Tooltip } from 'src/components/Tooltip';
+import useCSSTextTruncation from 'src/hooks/useTruncation/useCSSTextTruncation';
+import { FilterBarOrientation } from 'src/dashboard/types';
+import { updateDataMask } from 'src/dataMask/actions';
+
+const StyledCrossFilterTitle = styled.div`
+ ${({ theme }) => `
+ display: flex;
+ font-size: ${theme.typography.sizes.s}px;
+ color: ${theme.colors.grayscale.base};
+ vertical-align: middle;
+ cursor: pointer;
+ align-items: center;
+ `}
+`;
+const StyledIconSearch = styled(Icons.SearchOutlined)`
Review Comment:
Maybe we could add a transition to make changing the colour smoother?
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarCrossFilters/CrossFilter.tsx:
##########
@@ -0,0 +1,229 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { useCallback } from 'react';
+import { styled, t, css, useTheme } from '@superset-ui/core';
+import { CrossFilterIndicator } from 'src/dashboard/components/nativeFilters/selectors';
+import Icons from 'src/components/Icons';
+import { useDispatch } from 'react-redux';
+import { setFocusedNativeFilter } from 'src/dashboard/actions/nativeFilters';
+import { Tag } from 'src/components';
+import { Tooltip } from 'src/components/Tooltip';
+import useCSSTextTruncation from 'src/hooks/useTruncation/useCSSTextTruncation';
+import { FilterBarOrientation } from 'src/dashboard/types';
+import { updateDataMask } from 'src/dataMask/actions';
+
+const StyledCrossFilterTitle = styled.div`
+ ${({ theme }) => `
+ display: flex;
+ font-size: ${theme.typography.sizes.s}px;
+ color: ${theme.colors.grayscale.base};
+ vertical-align: middle;
+ cursor: pointer;
+ align-items: center;
+ `}
+`;
+const StyledIconSearch = styled(Icons.SearchOutlined)`
+ ${({ theme }) => `
+ color: ${theme.colors.grayscale.light1};
+ margin-left: ${theme.gridUnit * 2}px;
+ &:hover {
+ color: ${theme.colors.grayscale.base};
+ }
+ `}
+`;
+
+const ellipsisCss = css`
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: inline-block;
+ vertical-align: middle;
+`;
+
+const StyledCrossFilterValue = styled('b')`
Review Comment:
```suggestion
const StyledCrossFilterValue = styled.b`
```
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarCrossFilters/Vertical.tsx:
##########
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { useMemo } from 'react';
+import Collapse from 'src/components/Collapse';
+import { styled, t, DataMaskStateWithId } from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import {
+ DashboardInfo,
+ DashboardLayout,
+ FilterBarOrientation,
+ RootState,
+} from 'src/dashboard/types';
+import CrossFilter from './CrossFilter';
+import crossFiltersSelector from './selectors';
+
+const StyledCollapse = styled(Collapse)`
+ ${({ theme }) => `
+ .ant-collapse-item > .ant-collapse-header {
+ padding-bottom: 0;
+ }
+ .ant-collapse-item > .ant-collapse-header > .ant-collapse-arrow {
+ font-size: ${theme.typography.sizes.xs}px;
+ padding-top: ${theme.gridUnit * 3.5}px;
Review Comment:
Could we round it to 3 or 4? I think we agreed some time ago not to use fractions of grid unit
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarCrossFilters/CrossFilter.tsx:
##########
@@ -0,0 +1,229 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { useCallback } from 'react';
+import { styled, t, css, useTheme } from '@superset-ui/core';
+import { CrossFilterIndicator } from 'src/dashboard/components/nativeFilters/selectors';
+import Icons from 'src/components/Icons';
+import { useDispatch } from 'react-redux';
+import { setFocusedNativeFilter } from 'src/dashboard/actions/nativeFilters';
+import { Tag } from 'src/components';
+import { Tooltip } from 'src/components/Tooltip';
+import useCSSTextTruncation from 'src/hooks/useTruncation/useCSSTextTruncation';
+import { FilterBarOrientation } from 'src/dashboard/types';
+import { updateDataMask } from 'src/dataMask/actions';
+
+const StyledCrossFilterTitle = styled.div`
+ ${({ theme }) => `
+ display: flex;
+ font-size: ${theme.typography.sizes.s}px;
+ color: ${theme.colors.grayscale.base};
+ vertical-align: middle;
+ cursor: pointer;
+ align-items: center;
+ `}
+`;
+const StyledIconSearch = styled(Icons.SearchOutlined)`
+ ${({ theme }) => `
+ color: ${theme.colors.grayscale.light1};
+ margin-left: ${theme.gridUnit * 2}px;
+ &:hover {
+ color: ${theme.colors.grayscale.base};
+ }
+ `}
+`;
+
+const ellipsisCss = css`
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: inline-block;
+ vertical-align: middle;
+`;
+
+const StyledCrossFilterValue = styled('b')`
+ ${({ theme }) => `
+ max-width: ${theme.gridUnit * 25}px;
+ `}
+ ${ellipsisCss}
+`;
+
+const StyledCrossFilterColumn = styled('span')`
+ ${({ theme }) => `
+ max-width: ${theme.gridUnit * 25}px;
+ padding-right: ${theme.gridUnit}px;
+ `}
+ ${ellipsisCss}
+`;
+
+const CrossFilterTag = (props: {
+ filter: CrossFilterIndicator;
+ orientation: FilterBarOrientation;
+ removeCrossFilter: (filterId: number) => void;
+}) => {
+ const { filter, orientation, removeCrossFilter } = props;
+ const theme = useTheme();
+ const [columnRef, columnIsTruncated] =
+ useCSSTextTruncation<HTMLSpanElement>();
+ const [valueRef, valueIsTruncated] = useCSSTextTruncation<HTMLSpanElement>();
+
+ return (
+ <Tag
+ css={css`
+ ${orientation === FilterBarOrientation.VERTICAL
+ ? `
+ margin-top: ${theme.gridUnit * 2}px;
+ `
+ : `
+ margin-left: ${theme.gridUnit * 2}px;
+ `}
+ `}
+ closable
+ onClose={() => removeCrossFilter(filter.emitterId)}
+ >
+ <Tooltip title={columnIsTruncated ? filter.column : null}>
+ <StyledCrossFilterColumn ref={columnRef}>
+ {filter.column}
+ </StyledCrossFilterColumn>
+ </Tooltip>
+ <Tooltip title={valueIsTruncated ? filter.value : null}>
+ <StyledCrossFilterValue ref={valueRef}>
+ {filter.value}
+ </StyledCrossFilterValue>
+ </Tooltip>
+ </Tag>
+ );
+};
+
+const CrossFilterChartTitle = (props: {
+ title: string;
+ orientation: FilterBarOrientation;
+}) => {
+ const { title, orientation } = props;
+ const [titleRef, titleIsTruncated] = useCSSTextTruncation<HTMLSpanElement>();
+ const theme = useTheme();
+ return (
+ <Tooltip title={titleIsTruncated ? title : null}>
+ <span
+ css={css`
+ max-width: ${orientation === FilterBarOrientation.VERTICAL
+ ? `${theme.gridUnit * 60}px`
+ : `${theme.gridUnit * 15}px`};
+ line-height: 20px;
+ ${ellipsisCss}
+ `}
+ ref={titleRef}
+ >
+ {title}
+ </span>
+ </Tooltip>
+ );
+};
+
+const CrossFilter = (props: {
+ filter: CrossFilterIndicator;
+ orientation: FilterBarOrientation;
+ last?: boolean;
+}) => {
+ const { filter, orientation, last } = props;
+ const theme = useTheme();
+ const dispatch = useDispatch();
+
+ const onHighlightFilterSource = useCallback(
+ (path?: string[]) => {
+ if (path) {
+ dispatch(setFocusedNativeFilter(path[0]));
+ }
+ },
+ [dispatch],
+ );
+
+ const handleRemoveCrossFilter = (chartId: number) => {
+ dispatch(
+ updateDataMask(chartId, {
+ extraFormData: {
+ filters: [],
+ },
+ filterState: {
+ value: null,
+ selectedValues: null,
+ },
+ }),
+ );
+ };
+
+ return (
+ <div
+ key={`${filter.name}${filter.emitterId}`}
+ css={css`
+ ${orientation === FilterBarOrientation.VERTICAL
+ ? `
+ display: block;
+ margin-top: ${theme.gridUnit * 4}px;
+ `
+ : `
+ display: flex;
+ `}
+ `}
+ >
+ <StyledCrossFilterTitle
+ onClick={() => onHighlightFilterSource(filter.path)}
+ role="button"
+ tabIndex={0}
+ >
+ <CrossFilterChartTitle
+ title={filter.name}
+ orientation={orientation || FilterBarOrientation.HORIZONTAL}
+ />
+ <Tooltip title={t('Locate the chart')}>
+ <StyledIconSearch iconSize="s" />
+ </Tooltip>
+ </StyledCrossFilterTitle>
+ {(filter.column || filter.value) && (
+ <CrossFilterTag
+ filter={filter}
+ orientation={orientation}
+ removeCrossFilter={handleRemoveCrossFilter}
+ />
+ )}
+ {last && (
Review Comment:
I'm wondering if that separator line should be a part of `CrossFilter` component π€ Maybe we could extract it from here, render all cross filters in a container and then draw that line after the container? That would also let us get rid of that `last` logic
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1442057411
This is incredible stuff, love it! β€οΈ
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1441559561
@kgabryje thanks for the feedback! All above issues should be sorted now. I have also included unit tests and separated the code a little bit more.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1441634913
/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido merged pull request #23138: feat: Cross Filters in FilterBar
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido merged PR #23138:
URL: https://github.com/apache/superset/pull/23138
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1439798215
1 more comment (we can handle it in a separate PR) - currently we display the filter bar only if the native filters FF is enabled. I think now that we use the filter bar to also show the cross filters, we should display the filter bar when either of the feature flags is enabled
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1444316981
> I found an issue that when i try to locate the chart for emitting cross filter, all charts under a mask and the emitting chart is not highlight, i have to refresh page to show the charts again
@jinghua-qa it is a known issue mentioned in the PR description that @kgabryje has fixed and we are ready to merge.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1442016606
/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_HORIZONTAL_FILTER_BAR=true
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1441697419
A few more comments:
1. I think the search icon is not centered. I fixed it by giving the `span.anticon` `line-height: 0`.
<img width="187" alt="image" src="https://user-images.githubusercontent.com/15073128/220905877-0f78ff1f-8a6e-4247-8618-9e3a28b4120e.png">
2. The spacing between the X and the right edge of the tag is slightly too large - 8px instead of 4
<img width="228" alt="image" src="https://user-images.githubusercontent.com/15073128/220906743-4d93c5d3-0777-4bfc-af2f-a73f9a2803c7.png">
3. I think the separator line is redundant if there are no native filters added
<img width="475" alt="image" src="https://user-images.githubusercontent.com/15073128/220906572-f75ccd2d-8600-4ab9-ac75-a7c91a64f88b.png">
4. If there are no native filters, there's a vertical line in the "More filters" dropdown
<img width="353" alt="image" src="https://user-images.githubusercontent.com/15073128/220907003-f0943172-e8a0-49f8-a33f-28c69985a45c.png">
5. Not sure what to do about that empty state here...
<img width="311" alt="image" src="https://user-images.githubusercontent.com/15073128/220907229-82a65f4b-bdcb-4f04-b15d-b7db33430f42.png">
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1439770083
A few visual issues:
1. Tag's X button is not centered.
2. Tag's outline is different colour than in designs
<img width="326" alt="image" src="https://user-images.githubusercontent.com/15073128/220590393-703cfb9a-0b2c-4453-b383-ef441c7635b7.png">
<img width="420" alt="image" src="https://user-images.githubusercontent.com/15073128/220590445-ecffab08-acd5-474f-ba00-bce8d58f0c20.png">
3. Horizontal line between cross-filters and native filters is missing in vertical filter bar
<img width="607" alt="image" src="https://user-images.githubusercontent.com/15073128/220590590-ee13653a-dbc7-4db8-955d-794424a2ef37.png">
4. Spacing between title and icon is 8px, should be 4
<img width="113" alt="image" src="https://user-images.githubusercontent.com/15073128/220591551-524ef612-33f8-412b-be3c-c8a197603220.png">
5. Separator line in horizontal bar is too thick - should be 1px
<img width="143" alt="image" src="https://user-images.githubusercontent.com/15073128/220591678-d538535b-95c5-4fb8-bf8b-42821b9daa5f.png">
6. Top margin in overflow dropdown is too big
<img width="430" alt="image" src="https://user-images.githubusercontent.com/15073128/220591905-e63ba40c-9cf0-4fd2-a23d-3455e27fa828.png">
7. Separator line in overflow dropdown is too thick
<img width="379" alt="image" src="https://user-images.githubusercontent.com/15073128/220592147-cca6e59a-3078-4899-a2d4-b93279ebf5e7.png">
8. I think it wasn't included in designs, but "Filters out of scope" should probably be the same font size as "Cross-filters" collapsible title for consistency
<img width="378" alt="image" src="https://user-images.githubusercontent.com/15073128/220592433-a513bd6e-6229-4e78-989a-1e6b466a8734.png">
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1441641662
@kgabryje Ephemeral environment spinning up at http://54.189.152.184:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kasiazjc commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "kasiazjc (via GitHub)" <gi...@apache.org>.
kasiazjc commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1443487824
> @kasiazjc about the "No filters" empty state that @kgabryje mentioned above, that is referring to the native filters and it is mentioning the button to add new filters. Should we change the copy a bit to make sure it is referring to the native filters and not cross filters?
So this is what I'm thinking:
> No global filters are currently added
> Click on β+Add/Edit Filtersβ button to create new dashboard filters.
In the next version of the dashboard filters we wanted to actually use term "global filters" which made sense to our users and they could easy understand what does it mean. So let's start there
thanks for all the work @geido and @kgabryje β€οΈ
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1446196357
Ephemeral environment shutdown and build artifacts deleted.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1438669356
# [Codecov](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#23138](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e936866) into [master](https://codecov.io/gh/apache/superset/commit/f4ffed24babc8923cc812bc7bf3cb07ffd7e367a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f4ffed2) will **decrease** coverage by `11.32%`.
> The diff coverage is `39.13%`.
```diff
@@ Coverage Diff @@
## master #23138 +/- ##
===========================================
- Coverage 67.54% 56.23% -11.32%
===========================================
Files 1881 1878 -3
Lines 72392 72059 -333
Branches 7882 7801 -81
===========================================
- Hits 48898 40522 -8376
- Misses 21472 29535 +8063
+ Partials 2022 2002 -20
```
| Flag | Coverage Ξ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `52.66% <ΓΈ> (ΓΈ)` | |
| python | `58.87% <ΓΈ> (-23.40%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `52.47% <ΓΈ> (ΓΈ)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ | |
|---|---|---|
| [...ard/components/FiltersBadge/DetailsPanel/index.tsx](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0ZpbHRlcnNCYWRnZS9EZXRhaWxzUGFuZWwvaW5kZXgudHN4) | `82.22% <ΓΈ> (ΓΈ)` | |
| [.../components/FiltersBadge/FilterIndicator/index.tsx](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0ZpbHRlcnNCYWRnZS9GaWx0ZXJJbmRpY2F0b3IvaW5kZXgudHN4) | `87.50% <ΓΈ> (ΓΈ)` | |
| [...nd/src/dashboard/components/FiltersBadge/index.tsx](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0ZpbHRlcnNCYWRnZS9pbmRleC50c3g=) | `86.11% <ΓΈ> (ΓΈ)` | |
| [...ilters/FilterBar/FilterControls/FilterControls.tsx](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyQmFyL0ZpbHRlckNvbnRyb2xzL0ZpbHRlckNvbnRyb2xzLnRzeA==) | `63.26% <ΓΈ> (-1.55%)` | :arrow_down: |
| [...Filters/FilterBar/FiltersDropdownContent/index.tsx](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyQmFyL0ZpbHRlcnNEcm9wZG93bkNvbnRlbnQvaW5kZXgudHN4) | `25.00% <ΓΈ> (ΓΈ)` | |
| [.../components/nativeFilters/FilterBar/Horizontal.tsx](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyQmFyL0hvcml6b250YWwudHN4) | `100.00% <ΓΈ> (ΓΈ)` | |
| [...rc/dashboard/components/nativeFilters/selectors.ts](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvc2VsZWN0b3JzLnRz) | `68.14% <32.25%> (ΓΈ)` | |
| [...lters/FilterBar/FilterBarCrossFilters/Vertical.tsx](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyQmFyL0ZpbHRlckJhckNyb3NzRmlsdGVycy9WZXJ0aWNhbC50c3g=) | `40.00% <40.00%> (ΓΈ)` | |
| [...rd/components/nativeFilters/FilterBar/Vertical.tsx](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyQmFyL1ZlcnRpY2FsLnRzeA==) | `81.48% <60.00%> (+6.97%)` | :arrow_up: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [363 more](https://codecov.io/gh/apache/superset/pull/23138?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: Weβre building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] jinghua-qa commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "jinghua-qa (via GitHub)" <gi...@apache.org>.
jinghua-qa commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1444239203
I found an issue that when i try to locate the chart for emitting cross filter, all charts under a mask and the emitting chart is not highlight, i have to refresh page to show the charts again
https://user-images.githubusercontent.com/81597121/221263954-fec9b1cd-66f4-4d44-867b-e3311f3df689.mov
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1441662921
@kgabryje Ephemeral environment spinning up at http://35.87.42.24:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on pull request #23138: feat: Cross Filters in FilterBar
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23138:
URL: https://github.com/apache/superset/pull/23138#issuecomment-1441657939
/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_HORIZONTAL_FILTER_BAR=true
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on a diff in pull request #23138: feat: Cross Filters in FilterBar
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #23138:
URL: https://github.com/apache/superset/pull/23138#discussion_r1114155721
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarCrossFilters/CrossFilter.tsx:
##########
@@ -0,0 +1,229 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { useCallback } from 'react';
+import { styled, t, css, useTheme } from '@superset-ui/core';
+import { CrossFilterIndicator } from 'src/dashboard/components/nativeFilters/selectors';
+import Icons from 'src/components/Icons';
+import { useDispatch } from 'react-redux';
+import { setFocusedNativeFilter } from 'src/dashboard/actions/nativeFilters';
+import { Tag } from 'src/components';
+import { Tooltip } from 'src/components/Tooltip';
+import useCSSTextTruncation from 'src/hooks/useTruncation/useCSSTextTruncation';
+import { FilterBarOrientation } from 'src/dashboard/types';
+import { updateDataMask } from 'src/dataMask/actions';
+
+const StyledCrossFilterTitle = styled.div`
+ ${({ theme }) => `
+ display: flex;
+ font-size: ${theme.typography.sizes.s}px;
+ color: ${theme.colors.grayscale.base};
+ vertical-align: middle;
+ cursor: pointer;
+ align-items: center;
+ `}
+`;
+const StyledIconSearch = styled(Icons.SearchOutlined)`
+ ${({ theme }) => `
+ color: ${theme.colors.grayscale.light1};
+ margin-left: ${theme.gridUnit * 2}px;
+ &:hover {
+ color: ${theme.colors.grayscale.base};
+ }
+ `}
+`;
+
+const ellipsisCss = css`
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: inline-block;
+ vertical-align: middle;
+`;
+
+const StyledCrossFilterValue = styled('b')`
+ ${({ theme }) => `
+ max-width: ${theme.gridUnit * 25}px;
+ `}
+ ${ellipsisCss}
+`;
+
+const StyledCrossFilterColumn = styled('span')`
+ ${({ theme }) => `
+ max-width: ${theme.gridUnit * 25}px;
+ padding-right: ${theme.gridUnit}px;
+ `}
+ ${ellipsisCss}
+`;
+
+const CrossFilterTag = (props: {
+ filter: CrossFilterIndicator;
+ orientation: FilterBarOrientation;
+ removeCrossFilter: (filterId: number) => void;
+}) => {
+ const { filter, orientation, removeCrossFilter } = props;
+ const theme = useTheme();
+ const [columnRef, columnIsTruncated] =
+ useCSSTextTruncation<HTMLSpanElement>();
+ const [valueRef, valueIsTruncated] = useCSSTextTruncation<HTMLSpanElement>();
+
+ return (
+ <Tag
+ css={css`
+ ${orientation === FilterBarOrientation.VERTICAL
+ ? `
+ margin-top: ${theme.gridUnit * 2}px;
+ `
+ : `
+ margin-left: ${theme.gridUnit * 2}px;
+ `}
+ `}
+ closable
+ onClose={() => removeCrossFilter(filter.emitterId)}
+ >
+ <Tooltip title={columnIsTruncated ? filter.column : null}>
+ <StyledCrossFilterColumn ref={columnRef}>
+ {filter.column}
+ </StyledCrossFilterColumn>
+ </Tooltip>
+ <Tooltip title={valueIsTruncated ? filter.value : null}>
+ <StyledCrossFilterValue ref={valueRef}>
+ {filter.value}
+ </StyledCrossFilterValue>
+ </Tooltip>
+ </Tag>
+ );
+};
+
+const CrossFilterChartTitle = (props: {
+ title: string;
+ orientation: FilterBarOrientation;
+}) => {
+ const { title, orientation } = props;
+ const [titleRef, titleIsTruncated] = useCSSTextTruncation<HTMLSpanElement>();
+ const theme = useTheme();
+ return (
+ <Tooltip title={titleIsTruncated ? title : null}>
+ <span
+ css={css`
+ max-width: ${orientation === FilterBarOrientation.VERTICAL
+ ? `${theme.gridUnit * 60}px`
+ : `${theme.gridUnit * 15}px`};
+ line-height: 20px;
+ ${ellipsisCss}
+ `}
+ ref={titleRef}
+ >
+ {title}
+ </span>
+ </Tooltip>
+ );
+};
+
+const CrossFilter = (props: {
+ filter: CrossFilterIndicator;
+ orientation: FilterBarOrientation;
+ last?: boolean;
+}) => {
+ const { filter, orientation, last } = props;
+ const theme = useTheme();
+ const dispatch = useDispatch();
+
+ const onHighlightFilterSource = useCallback(
+ (path?: string[]) => {
+ if (path) {
+ dispatch(setFocusedNativeFilter(path[0]));
+ }
+ },
+ [dispatch],
+ );
+
+ const handleRemoveCrossFilter = (chartId: number) => {
+ dispatch(
+ updateDataMask(chartId, {
+ extraFormData: {
+ filters: [],
+ },
+ filterState: {
+ value: null,
+ selectedValues: null,
+ },
+ }),
+ );
+ };
+
+ return (
+ <div
+ key={`${filter.name}${filter.emitterId}`}
+ css={css`
+ ${orientation === FilterBarOrientation.VERTICAL
+ ? `
+ display: block;
+ margin-top: ${theme.gridUnit * 4}px;
+ `
+ : `
+ display: flex;
+ `}
+ `}
+ >
+ <StyledCrossFilterTitle
+ onClick={() => onHighlightFilterSource(filter.path)}
+ role="button"
+ tabIndex={0}
+ >
+ <CrossFilterChartTitle
+ title={filter.name}
+ orientation={orientation || FilterBarOrientation.HORIZONTAL}
+ />
+ <Tooltip title={t('Locate the chart')}>
+ <StyledIconSearch iconSize="s" />
+ </Tooltip>
+ </StyledCrossFilterTitle>
+ {(filter.column || filter.value) && (
+ <CrossFilterTag
+ filter={filter}
+ orientation={orientation}
+ removeCrossFilter={handleRemoveCrossFilter}
+ />
+ )}
+ {last && (
Review Comment:
The reason behind this choice is that when the cross filters are displayed in horizontal mode, we need to account for both displaying them in the filter bar as well as in "more filters" dropdown. Both use the component directly to display each filter similarly to what we do with native filters. Creating a container around this logic would make the rendering function more complex and introduce a different pattern in the filter controls.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org