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 2022/05/06 17:42:17 UTC
[GitHub] [superset] cccs-RyanK opened a new pull request, #19979: [WIP] feat(chart-explore): add select all functionality to table viz type
cccs-RyanK opened a new pull request, #19979:
URL: https://github.com/apache/superset/pull/19979
<!---
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 -->
The select all functionality was removed previously, but our users rely on this useful feature in the explore chart, so this PR is for bringing back that feature.
With the way things currently are, the user must manually select columns one by one by clicking or drag and drop into the columns field when building a query in the explore view. When the user is initially creating a chart, it is often the case that they want to query all the possible fields to identify what is relevant and narrow it down. This becomes tedious when dealing with data-sets that have many columns (100+).
We have had to create a temporary solution in our fork just to allow our users who rely on it to use the explore chart. This design aims to reintroduce the select all functionality while keeping the UX clean. It is only shown in the columns select when in the raw records query mode in the explore view on the 'table plugin', since this is the place where it is needed by our users.
Thoughts and feedback are appreciated!
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
Drag and Drop Feature Enabled:
select all
When the columns field is empty, the select all button can be clicked to populate the columns values with all available options.
deselect all
Once any option(s) have been selected, the button text changes to deselect all. If it is clicked in this state, it empties the columns value (and changes back to 'select all').
Drag and Drop Feature Disabled
* the selected values component is "full" when all options are currently selected (list of selected values equals list of options)
select all
When the columns value is not full*, the select all button can be clicked to add all remaining options. Since there is already a 'clear' button built in to select control when dnd is disabled, the select all button does not turn into a deselect all button when an item is added. This allows a user to select all options without first clearing the values and then selecting all. The user can still clear the values at any time with the 'clear' button.
select all disabled
When the columns value is full*, the select all button is disabled because all options are currently selected.
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
Drag and Drop Feature enabled
- Go into the explore view for a chart that has the 'Table' viz type.
- In the 'Data' tab, under the 'Query' drop down, select 'Raw Records' as the 'Query Mode'.
- For the 'Columns' selector, you should see a 'Select All' or 'Deselect All' button.
- Clicking it should select all options when the button is in select all mode, and should deselect all buttons when in deselect mode.
Drag and Drop Feature disabled
- Go into the explore view for a chart that has the 'Table' viz type.
- In the 'Data' tab, under the 'Query' drop down, select 'Raw Records' as the 'Query Mode'.
- For the 'Columns' selector, you should see a 'Select All' button.
- If all options are selected, the button should be disabled.
- Otherwise, the button should be enabled and should select all remaining options
- The clear button in the selector component should still clear all selected values.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: https://github.com/apache/superset/discussions/18247
- [ ] Required feature flags:
- [x] 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
- [x] 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] SharonCastel commented on pull request #19979: feat(chart-explore): add select all functionality to table viz type
Posted by GitBox <gi...@apache.org>.
SharonCastel commented on PR #19979:
URL: https://github.com/apache/superset/pull/19979#issuecomment-1219621611
I think that it can be expressed as * as well on the saved expressions
I have tables of 150-250 columns and its a nightmare for me,
please keep on the hard work and add it to the next version.
--
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] cccs-RyanK closed pull request #19979: feat(chart-explore): add select all functionality to table viz type
Posted by GitBox <gi...@apache.org>.
cccs-RyanK closed pull request #19979: feat(chart-explore): add select all functionality to table viz type
URL: https://github.com/apache/superset/pull/19979
--
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] ktmud commented on a diff in pull request #19979: feat(chart-explore): add select all functionality to table viz type
Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #19979:
URL: https://github.com/apache/superset/pull/19979#discussion_r867247847
##########
superset-frontend/src/explore/components/controls/SelectControl.jsx:
##########
@@ -160,6 +164,17 @@ export default class SelectControl extends React.PureComponent {
return filterOption({ data: option }, text);
}
+ handleSelectAll() {
+ /**
+ * Function to handle select all button.
+ * Adds all options to the selected values.
+ */
Review Comment:
```suggestion
/**
* Function to handle select all button.
* Adds all options to the selected values.
*/
handleSelectAll() {
```
Nit: normally we put function doc string above the function name.
##########
superset-frontend/src/explore/components/SelectAllButton/index.tsx:
##########
@@ -0,0 +1,49 @@
+/**
+ * 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 from 'react';
+
+import { t, css, SupersetTheme } from '@superset-ui/core';
+import Button, { OnClickHandler } from 'src/components/Button';
+
+export type SelectAllButtonProps = {
+ onSelectAll: OnClickHandler;
+ selectMode: () => boolean;
+ deselectEnabled: boolean;
+};
+
+export const SelectAllButton = ({
+ onSelectAll,
+ selectMode = () => true,
+ deselectEnabled = false,
+ }: SelectAllButtonProps) =>
+ (
+ <Button
+ buttonStyle={'link'}
+ disabled={deselectEnabled ? false : !selectMode()}
+ css={(theme: SupersetTheme) =>
+ css`
+ padding: 0;
+ `
+ }
+ onClick={onSelectAll}
+ >
+ {deselectEnabled && !selectMode()? t('Deselect All'): t('Select All')}
Review Comment:
```suggestion
{deselectEnabled && !selectMode()? t('Deselect all'): t('Select all')}
```
We use sentence case for control buttons. Although in this case it doesn't matter since it's all capitalized by CSS.
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx:
##########
@@ -229,6 +271,7 @@ export function DndColumnSelect(props: DndColumnSelectProps) {
? openPopover
: undefined
}
+ selectAllButton={showSelectAllButton ? <SelectAllButton {...selectAllButtonProps} /> : null}
Review Comment:
```suggestion
selectAllButton={
showSelectAllButton ? (
<Button
buttonStyle="link"
disabled={hasSelectedAll}
onClick={
hasSelectedAll
? undefined
: () => hasSelectedSome ? deselectAll() : selectAll()
}
>
{hasSelectedSome ? t('Deselect all') : t('Select all')}
</Button>
) : null
}
```
I think it's possible to directly render the button without all the "mode" conversion. This should make things easier to understand.
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx:
##########
@@ -43,11 +43,17 @@ export type DndSelectLabelProps = {
valuesRenderer: () => ReactNode;
displayGhostButton?: boolean;
onClickGhostButton?: () => void;
+ /**
+ * Adds a select all button next to the header, allowing the user to
+ * select all options in one click. Can only be a SelectAllButton or null.
+ */
+ selectAllButton?: ReactNode;
Review Comment:
```suggestion
headerAfter?: ReactNode;
```
I think it's okay to make this node more generic. You never know if you want to add a different type of control here. For example, in non-DnD mode adhoc metrics control, this is a plus button.
##########
superset-frontend/src/explore/components/SelectAllButton/index.tsx:
##########
@@ -0,0 +1,49 @@
+/**
+ * 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 from 'react';
+
+import { t, css, SupersetTheme } from '@superset-ui/core';
+import Button, { OnClickHandler } from 'src/components/Button';
+
+export type SelectAllButtonProps = {
+ onSelectAll: OnClickHandler;
+ selectMode: () => boolean;
+ deselectEnabled: boolean;
+};
+
+export const SelectAllButton = ({
+ onSelectAll,
+ selectMode = () => true,
+ deselectEnabled = false,
+ }: SelectAllButtonProps) =>
+ (
+ <Button
+ buttonStyle={'link'}
+ disabled={deselectEnabled ? false : !selectMode()}
+ css={(theme: SupersetTheme) =>
+ css`
+ padding: 0;
+ `
+ }
Review Comment:
```suggestion
css={{ padding: 0 }}
```
Since `theme` is not used, this should be good enough.
##########
superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx:
##########
@@ -118,6 +118,7 @@ const all_columns: typeof sharedControls.groupby = {
}),
visibility: isRawMode,
resetOnHide: false,
+ showSelectAllButton: true,
Review Comment:
```suggestion
showSelectAll: true,
```
I think it's okay to call this prop `showSelectAll` in case we want to change how the "select all" trigger looks like.
--
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 #19979: feat(chart-explore): add select all functionality to table viz type
Posted by GitBox <gi...@apache.org>.
kasiazjc commented on PR #19979:
URL: https://github.com/apache/superset/pull/19979#issuecomment-1121130960
Hi @cccs-RyanK! Wanted to let you know that we are currently working on adding "select all" feature to all of the multiple choice select fields (besides dnd) which should be available soon (probably 2-3 weeks). Drag&drop select all is something that I hope we will tackle in the upcoming weeks so there will be a better solution provided from our side.
Thank you for helping an developing this solution 🙏
--
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] cccs-rc commented on pull request #19979: feat(chart-explore): add select all functionality to table viz type
Posted by GitBox <gi...@apache.org>.
cccs-rc commented on PR #19979:
URL: https://github.com/apache/superset/pull/19979#issuecomment-1121265575
> Hi @cccs-RyanK! Wanted to let you know that we are currently working on adding "select all" feature to all of the multiple choice select fields (besides dnd) which should be available soon (probably 2-3 weeks). Drag&drop select all is something that I hope we will tackle in the upcoming weeks so there will be a better solution provided from our side.
>
> Thank you for helping an developing this solution 🙏
Hi @kasiazjc! Thanks for the info! Is there anywhere we can track what features are being actively worked on from your end? We'd like to avoid duplicating effort if we can!
--
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 #19979: feat(chart-explore): add select all functionality to table viz type
Posted by GitBox <gi...@apache.org>.
kasiazjc commented on PR #19979:
URL: https://github.com/apache/superset/pull/19979#issuecomment-1122217111
> > Hi @cccs-RyanK! Wanted to let you know that we are currently working on adding "select all" feature to all of the multiple choice select fields (besides dnd) which should be available soon (probably 2-3 weeks). Drag&drop select all is something that I hope we will tackle in the upcoming weeks so there will be a better solution provided from our side.
> > Thank you for helping an developing this solution 🙏
>
> Hi @kasiazjc! Thanks for the info! Is there anywhere we can track what features are being actively worked on from your end? We'd like to avoid duplicating effort if we can!
Unfortunately there is no way to track it right now 😢 we're trying to find a way to sort it out, but it may not happen anytime soon. I will try my best to keep you updated for now about this one 🤞
--
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