You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/07/23 16:31:47 UTC
[GitHub] [superset] stephenLYZ opened a new pull request #15868: chore: implement new mockup to the new viz gallery
stephenLYZ opened a new pull request #15868:
URL: https://github.com/apache/superset/pull/15868
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
This PR is the second iteration of viz gallery.
- To relocate the search bar to main section
- Search to take up the entire row
- Stack tags above category list, only display `Highly-used`, `ECharts`, `Advanced-Analytics`
- Click to expand and show 4 sections:
- recommended tags
- all
- category
- tags
- from global new button - anchor to show recommended tags (implemented)
- from explore(switching viz) -anchor to show existing chart and its category (implemented)
- open from dataset list -> explore -> anchor to show recommended tags (not implemented)
related: https://github.com/apache/superset/pull/15734
design: https://www.figma.com/file/LMnulBPitqSh5GeElVbsUV/chart-popover?node-id=0%3A1
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
If text overflow is displayed tooltip.
show existing chart and scroll to the category, enable the user to see the selected category.
https://user-images.githubusercontent.com/11830681/126812550-aa57485b-72b5-4461-9dd6-14c5e1b8fb08.mov
### 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:
- [ ] 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 #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886918108
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] stephenLYZ commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
stephenLYZ commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886185748
/testenv up
--
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] suddjian commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886893016
I would like to see the "All Charts" section moved to the top and outside of a Collapse. Thoughts @junlincc ?
--
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] suddjian commented on a change in pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #15868:
URL: https://github.com/apache/superset/pull/15868#discussion_r675770478
##########
File path: superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx
##########
@@ -469,83 +578,147 @@ export default function VizTypeGallery(props: VizTypeGalleryProps) {
searchInputRef.current!.blur();
}, []);
- const selectCategory = useCallback(
+ const clickSelector = useCallback(
(key: string) => {
if (isSearchFocused) {
stopSearching();
}
- setActiveCategory(key);
+ setActiveSelector(key);
// clear the selected viz if it is not present in the new category
const isSelectedVizCompatible =
selectedVizMetadata && doesVizMatchCategory(selectedVizMetadata, key);
- if (key !== activeCategory && !isSelectedVizCompatible) {
+ if (key !== activeSelector && !isSelectedVizCompatible) {
onChange(null);
}
},
[
stopSearching,
isSearchFocused,
- activeCategory,
+ activeSelector,
selectedVizMetadata,
onChange,
],
);
+ const clearSelector = useCallback(e => {
+ e.stopPropagation();
+ if (isSearchFocused) {
+ stopSearching();
+ }
+ // clear current selector and set all charts
+ setActiveSelector(ALL_CHARTS);
+ }, []);
+
const vizEntriesToDisplay = isActivelySearching
? searchResults
- : chartsByCategory[activeCategory] || [];
+ : activeSelector === ALL_CHARTS
+ ? sortedMetadata
+ : chartsByCategory[activeSelector] || chartsByTags[activeSelector] || [];
return (
<VizPickerLayout className={className}>
<LeftPane>
- <SearchWrapper>
- <Input
- type="text"
- ref={searchInputRef as any /* cast required because emotion */}
- value={searchInputValue}
- placeholder={t('Search')}
- onChange={changeSearch}
- onFocus={focusSearch}
- data-test={`${VIZ_TYPE_CONTROL_TEST_ID}__search-input`}
- prefix={
- <InputIconAlignment>
- <Icons.Search iconSize="m" />
- </InputIconAlignment>
- }
- suffix={
- <InputIconAlignment>
- {searchInputValue && (
- <Icons.XLarge iconSize="m" onClick={stopSearching} />
- )}
- </InputIconAlignment>
- }
- />
- </SearchWrapper>
- <CategoriesWrapper>
- {categories.map(category => (
- <CategorySelector
- key={category}
- category={category}
- isSelected={!isActivelySearching && category === activeCategory}
- onClick={selectCategory}
- />
- ))}
- </CategoriesWrapper>
+ <Wrapper>
+ <Collapse
+ expandIconPosition="right"
+ ghost
+ defaultActiveKey={Object.values(COLLAPSE_LABEL)}
+ >
+ <Collapse.Panel
Review comment:
Might be able to reduce some duplication by making an array of selector groups and `map`ing that to render a collapse panel for each group.
--
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] stephenLYZ commented on a change in pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
stephenLYZ commented on a change in pull request #15868:
URL: https://github.com/apache/superset/pull/15868#discussion_r675726195
##########
File path: superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx
##########
@@ -144,25 +159,30 @@ const RightPane = styled.div`
overflow-y: scroll;
`;
-const AllTagsWrapper = styled.div`
- ${({ theme }) => `
- grid-area: tags;
- margin: ${theme.gridUnit * 4}px ${theme.gridUnit * 2}px 0;
- input {
- font-size: ${theme.typography.sizes.s};
- }
- `}
-`;
-
-const CategoriesWrapper = styled.div`
+const Wrapper = styled.div`
Review comment:
I think it's possible, I'll try 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] kgabryje commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886897950
1 more thing - we have an "X" button on "All charts" label, which doesn't do anything. Maybe "All charts" shouldn't have that button?
--
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] suddjian commented on a change in pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #15868:
URL: https://github.com/apache/superset/pull/15868#discussion_r675713200
##########
File path: superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx
##########
@@ -144,25 +159,30 @@ const RightPane = styled.div`
overflow-y: scroll;
`;
-const AllTagsWrapper = styled.div`
- ${({ theme }) => `
- grid-area: tags;
- margin: ${theme.gridUnit * 4}px ${theme.gridUnit * 2}px 0;
- input {
- font-size: ${theme.typography.sizes.s};
- }
- `}
-`;
-
-const CategoriesWrapper = styled.div`
+const Wrapper = styled.div`
Review comment:
`Wrapper` isn't a very descriptive name. I was going to suggest renaming it so that it indicates what it is wrapping, but then I noticed that it lives directly within the `LeftPane`. Is it possible to merge these styles into `LeftPane` and remove the `Wrapper` component?
--
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 #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886880417
Looks nice! 1 nit which is probably not related to this PR - there's a short vertical scroll in some charts' descriptions which looks a bit weird. I wonder if we could get rid of it somehow (shorter descriptions? fewer tags? larger modal?)
https://user-images.githubusercontent.com/15073128/127030663-55f73154-c62a-4b4f-b759-baef1bcf5bfb.mov
cc @junlincc
--
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] suddjian commented on a change in pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #15868:
URL: https://github.com/apache/superset/pull/15868#discussion_r675739658
##########
File path: superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx
##########
@@ -469,83 +578,147 @@ export default function VizTypeGallery(props: VizTypeGalleryProps) {
searchInputRef.current!.blur();
}, []);
- const selectCategory = useCallback(
+ const clickSelector = useCallback(
(key: string) => {
if (isSearchFocused) {
stopSearching();
}
- setActiveCategory(key);
+ setActiveSelector(key);
// clear the selected viz if it is not present in the new category
const isSelectedVizCompatible =
selectedVizMetadata && doesVizMatchCategory(selectedVizMetadata, key);
- if (key !== activeCategory && !isSelectedVizCompatible) {
+ if (key !== activeSelector && !isSelectedVizCompatible) {
onChange(null);
}
},
[
stopSearching,
isSearchFocused,
- activeCategory,
+ activeSelector,
selectedVizMetadata,
onChange,
],
);
+ const clearSelector = useCallback(e => {
+ e.stopPropagation();
+ if (isSearchFocused) {
+ stopSearching();
+ }
+ // clear current selector and set all charts
+ setActiveSelector(ALL_CHARTS);
+ }, []);
+
const vizEntriesToDisplay = isActivelySearching
? searchResults
- : chartsByCategory[activeCategory] || [];
+ : activeSelector === ALL_CHARTS
+ ? sortedMetadata
+ : chartsByCategory[activeSelector] || chartsByTags[activeSelector] || [];
return (
<VizPickerLayout className={className}>
<LeftPane>
- <SearchWrapper>
- <Input
- type="text"
- ref={searchInputRef as any /* cast required because emotion */}
- value={searchInputValue}
- placeholder={t('Search')}
- onChange={changeSearch}
- onFocus={focusSearch}
- data-test={`${VIZ_TYPE_CONTROL_TEST_ID}__search-input`}
- prefix={
- <InputIconAlignment>
- <Icons.Search iconSize="m" />
- </InputIconAlignment>
- }
- suffix={
- <InputIconAlignment>
- {searchInputValue && (
- <Icons.XLarge iconSize="m" onClick={stopSearching} />
- )}
- </InputIconAlignment>
- }
- />
- </SearchWrapper>
- <CategoriesWrapper>
- {categories.map(category => (
- <CategorySelector
- key={category}
- category={category}
- isSelected={!isActivelySearching && category === activeCategory}
- onClick={selectCategory}
- />
- ))}
- </CategoriesWrapper>
+ <Wrapper>
+ <Collapse
+ expandIconPosition="right"
+ ghost
+ defaultActiveKey={Object.values(COLLAPSE_LABEL)}
+ >
+ <Collapse.Panel
+ header={
+ <span className="header">
+ {COLLAPSE_LABEL.RECOMMENDED_TAGS}
+ </span>
+ }
+ key={COLLAPSE_LABEL.RECOMMENDED_TAGS}
+ >
+ {RECOMMENDED_TAGS.map(tag => (
+ <Selector
+ key={tag}
+ selector={tag}
+ type="Tags"
+ isSelected={!isActivelySearching && tag === activeSelector}
+ onClick={clickSelector}
+ onClear={clearSelector}
+ />
+ ))}
+ </Collapse.Panel>
+ <Collapse.Panel
+ header={
+ <span className="header">{COLLAPSE_LABEL.ALL_CHARTS}</span>
+ }
+ key={COLLAPSE_LABEL.ALL_CHARTS}
+ >
+ <Selector
Review comment:
It's a bit strange to have a collapse panel that will only ever have one item in it. Should we put the "All Charts" selector at the top of the left pane instead, outside of the collapse?
--
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] suddjian merged pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian merged pull request #15868:
URL: https://github.com/apache/superset/pull/15868
--
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] suddjian edited a comment on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian edited a comment on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-885786055
Suggestion, non-blocking:
Is `Advanced-Analytics` the longest tag we're dealing with? It looks like it should be able to fit within the left pane. Perhaps we can have the left pane size itself to fit its contents to avoid the need for a popover.
--
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 #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886185788
@stephenLYZ Ephemeral environment creation is currently limited to committers.
--
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] suddjian commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886858502
/testenv up
--
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] junlincc commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886898743
πLet's make above changes in a follow up PR>
--
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] suddjian commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-885936481
Played with this in the ephemeral env, looks good! Awesome work.
--
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] junlincc commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-885961885
@suddjian thanks for the review!!
--
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] suddjian commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-885787096
/testenv up
--
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] suddjian commented on a change in pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #15868:
URL: https://github.com/apache/superset/pull/15868#discussion_r675795296
##########
File path: superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx
##########
@@ -469,83 +578,147 @@ export default function VizTypeGallery(props: VizTypeGalleryProps) {
searchInputRef.current!.blur();
}, []);
- const selectCategory = useCallback(
+ const clickSelector = useCallback(
(key: string) => {
if (isSearchFocused) {
stopSearching();
}
- setActiveCategory(key);
+ setActiveSelector(key);
// clear the selected viz if it is not present in the new category
const isSelectedVizCompatible =
selectedVizMetadata && doesVizMatchCategory(selectedVizMetadata, key);
- if (key !== activeCategory && !isSelectedVizCompatible) {
+ if (key !== activeSelector && !isSelectedVizCompatible) {
onChange(null);
}
},
[
stopSearching,
isSearchFocused,
- activeCategory,
+ activeSelector,
selectedVizMetadata,
onChange,
],
);
+ const clearSelector = useCallback(e => {
+ e.stopPropagation();
+ if (isSearchFocused) {
+ stopSearching();
+ }
+ // clear current selector and set all charts
+ setActiveSelector(ALL_CHARTS);
+ }, []);
+
const vizEntriesToDisplay = isActivelySearching
? searchResults
- : chartsByCategory[activeCategory] || [];
+ : activeSelector === ALL_CHARTS
+ ? sortedMetadata
+ : chartsByCategory[activeSelector] || chartsByTags[activeSelector] || [];
return (
<VizPickerLayout className={className}>
<LeftPane>
- <SearchWrapper>
- <Input
- type="text"
- ref={searchInputRef as any /* cast required because emotion */}
- value={searchInputValue}
- placeholder={t('Search')}
- onChange={changeSearch}
- onFocus={focusSearch}
- data-test={`${VIZ_TYPE_CONTROL_TEST_ID}__search-input`}
- prefix={
- <InputIconAlignment>
- <Icons.Search iconSize="m" />
- </InputIconAlignment>
- }
- suffix={
- <InputIconAlignment>
- {searchInputValue && (
- <Icons.XLarge iconSize="m" onClick={stopSearching} />
- )}
- </InputIconAlignment>
- }
- />
- </SearchWrapper>
- <CategoriesWrapper>
- {categories.map(category => (
- <CategorySelector
- key={category}
- category={category}
- isSelected={!isActivelySearching && category === activeCategory}
- onClick={selectCategory}
- />
- ))}
- </CategoriesWrapper>
+ <Wrapper>
+ <Collapse
+ expandIconPosition="right"
+ ghost
+ defaultActiveKey={Object.values(COLLAPSE_LABEL)}
+ >
+ <Collapse.Panel
Review comment:
Or actually I think a better way to reduce duplication would be adding a component `<SelectorGroup>` that renders a Collapse Panel and its contents.
--
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 #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886074445
# [Codecov](https://codecov.io/gh/apache/superset/pull/15868?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 [#15868](https://codecov.io/gh/apache/superset/pull/15868?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fe8120f) into [master](https://codecov.io/gh/apache/superset/commit/d408ff8466b11c12de6cae69e7ca872c50cf5506?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d408ff8) will **increase** coverage by `0.01%`.
> The diff coverage is `82.66%`.
> :exclamation: Current head fe8120f differs from pull request most recent head 6a49026. Consider uploading reports for the commit 6a49026 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/15868?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15868 +/- ##
==========================================
+ Coverage 77.08% 77.09% +0.01%
==========================================
Files 984 984
Lines 51787 51819 +32
Branches 7031 7043 +12
==========================================
+ Hits 39918 39949 +31
Misses 11644 11644
- Partials 225 226 +1
```
| Flag | Coverage Ξ | |
|---|---|---|
| javascript | `71.75% <85.91%> (+0.02%)` | :arrow_up: |
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/15868?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ | |
|---|---|---|
| [superset-frontend/src/components/Icons/index.tsx](https://codecov.io/gh/apache/superset/pull/15868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvbnMvaW5kZXgudHN4) | `100.00% <ΓΈ> (ΓΈ)` | |
| [...frontend/src/views/CRUD/alert/AlertReportModal.tsx](https://codecov.io/gh/apache/superset/pull/15868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvQWxlcnRSZXBvcnRNb2RhbC50c3g=) | `61.70% <ΓΈ> (ΓΈ)` | |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/15868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `93.18% <ΓΈ> (ΓΈ)` | |
| [superset/views/base.py](https://codecov.io/gh/apache/superset/pull/15868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvYmFzZS5weQ==) | `75.86% <ΓΈ> (ΓΈ)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/15868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.21% <0.00%> (+0.16%)` | :arrow_up: |
| [...c/components/ErrorMessage/DatabaseErrorMessage.tsx](https://codecov.io/gh/apache/superset/pull/15868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRXJyb3JNZXNzYWdlL0RhdGFiYXNlRXJyb3JNZXNzYWdlLnRzeA==) | `90.90% <83.33%> (-3.83%)` | :arrow_down: |
| [...ponents/controls/VizTypeControl/VizTypeGallery.tsx](https://codecov.io/gh/apache/superset/pull/15868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9WaXpUeXBlQ29udHJvbC9WaXpUeXBlR2FsbGVyeS50c3g=) | `93.65% <86.15%> (-0.62%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/15868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.24% <100.00%> (ΓΈ)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/15868?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Ξ = absolute <relative> (impact)`, `ΓΈ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/15868?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d408ff8...6a49026](https://codecov.io/gh/apache/superset/pull/15868?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] stephenLYZ removed a comment on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
stephenLYZ removed a comment on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886185748
/testenv up
--
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 #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-885788191
@suddjian Ephemeral environment spinning up at http://54.188.201.103: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] github-actions[bot] commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-886861072
@suddjian Ephemeral environment spinning up at http://34.212.3.120: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] suddjian commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-885786055
Suggestion, non-blocking:
Is `Advanced-Analytics` the longest tag we're dealing with? It looks like it should be able to fit within the sidebar. Perhaps we can have the sidebar size itself to fit its contents to avoid the need for a popover.
--
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] stephenLYZ commented on pull request #15868: chore: implement new mockup to the new viz gallery (2nd iteration)
Posted by GitBox <gi...@apache.org>.
stephenLYZ commented on pull request #15868:
URL: https://github.com/apache/superset/pull/15868#issuecomment-885790708
> Suggestion, non-blocking:
> Is `Advanced-Analytics` the longest tag we're dealing with? It looks like it should be able to fit within the sidebar. Perhaps we can have the sidebar size itself to fit its contents to avoid the need for a popover.
Good idea! I agree that we do not limit the sidebar size, and we also don't need tooltip.
--
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