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/08/06 15:52:18 UTC
[GitHub] [superset] kgabryje opened a new pull request #16119: feat(explore): make dnd controls clickable
kgabryje opened a new pull request #16119:
URL: https://github.com/apache/superset/pull/16119
### SUMMARY
Add clickable labels and ghost buttons to drag and drop controls
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
https://user-images.githubusercontent.com/15073128/128537355-321742e4-069f-467a-a553-87b879f3ccae.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
CC @junlincc @mistercrunch
--
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 #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-896697739
I've hidden the new features behind a new feature flag `ENABLE_DND_WITH_CLICK_UX`.
@ktmud We recently added a suffix "here" to the ghost buttons
However, if there are more votes for a longer text "Click or drag a metric/column here to add" I can change that.
@junlincc WDYT?
--
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (37e6a91) into [master](https://codecov.io/gh/apache/superset/commit/ddb500590072ede3349a2a85a5d28a7953e21e32?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ddb5005) will **decrease** coverage by `0.29%`.
> The diff coverage is `37.16%`.
> :exclamation: Current head 37e6a91 differs from pull request most recent head ebe574d. Consider uploading reports for the commit ebe574d to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.83% 76.54% -0.30%
==========================================
Files 995 997 +2
Lines 52884 52995 +111
Branches 6721 6743 +22
==========================================
- Hits 40636 40563 -73
- Misses 12023 12207 +184
Partials 225 225
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| javascript | `71.07% <37.16%> (-0.16%)` | :arrow_down: |
| presto | `?` | |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `85.00% <ø> (ø)` | |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.82% <33.33%> (-0.25%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.89% <33.33%> (-0.18%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.00% <60.86%> (+1.57%)` | :arrow_up: |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `88.88% <88.88%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.49%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [11 more](https://codecov.io/gh/apache/superset/pull/16119/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [ddb5005...ebe574d](https://codecov.io/gh/apache/superset/pull/16119?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] junlincc edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-897119895
It depends on which direction we want to guide user to be on.
Here's my concern. With the + sign visual indication and the "Click or drop a metric/column here" placeholder, I am almost 100% sure first time users will click, instead of visit the left data panel first as their first interaction with this popover.
Once user start on one path, and find it sufficient enough, it's very likely they stick with the more familiar path and reluctant to try the other, which may slowdown the adoption of dnd.
The fact that DND offers a much fast time to chart, compared to click-to-select, I do want to intentionally guide user to start with this path, and promote this path.
I prefer to keep it simple"Drop a metric/column here" as the "+" is a loud enough call to action for clicking.
If we have to explicitly indicate again, "Click or drop a metric/column here" makes more sense to me, definitely not to use "drag" in the wording.
Also, recently we do identify some sever regressions in the control panel due to increasing complexity of the product by adding too many FF and having multiple environments. I can't emphasize enough that maintaining both and making sure both work well together and individually can be costly and messy.
I would push much harder keeping DND only and try to resolve all the existing concerns, bottlenecks by improving this one solution if I am working on a close source/design heavy product. However, Superset has its unique OSS DNA which should be preserved and respected.
To whomever approve and support this hybrid solution, please do assess the code, and test this PR throughout, and consider the risk, the complexity for long term. @ktmud @villebro
--
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 #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (37e6a91) into [master](https://codecov.io/gh/apache/superset/commit/772da8de6353f01ea1c64037af9695d5f10b71e4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (772da8d) will **decrease** coverage by `0.08%`.
> The diff coverage is `37.16%`.
> :exclamation: Current head 37e6a91 differs from pull request most recent head 702bbf4. Consider uploading reports for the commit 702bbf4 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.62% 76.54% -0.09%
==========================================
Files 995 997 +2
Lines 52886 52995 +109
Branches 6721 6743 +22
==========================================
+ Hits 40522 40563 +41
- Misses 12139 12207 +68
Partials 225 225
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.07% <37.16%> (-0.15%)` | :arrow_down: |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `85.00% <ø> (ø)` | |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.82% <33.33%> (-0.25%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.89% <33.33%> (-0.18%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.00% <60.86%> (+1.57%)` | :arrow_up: |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `88.88% <88.88%> (ø)` | |
| [...ols/DndColumnSelectControl/utils/optionSelector.ts](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL3V0aWxzL29wdGlvblNlbGVjdG9yLnRz) | `46.15% <0.00%> (+11.53%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [772da8d...702bbf4](https://codecov.io/gh/apache/superset/pull/16119?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] rusackas commented on a change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r688815983
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,223 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
Review comment:
Is this styling something we'd anticipate reusing anywhere? If so, maybe it should be styled via a prop on the Select component itself.
--
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bea2d00) into [master](https://codecov.io/gh/apache/superset/commit/ddb500590072ede3349a2a85a5d28a7953e21e32?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ddb5005) will **decrease** coverage by `0.08%`.
> The diff coverage is `37.71%`.
> :exclamation: Current head bea2d00 differs from pull request most recent head ebe574d. Consider uploading reports for the commit ebe574d to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.83% 76.75% -0.09%
==========================================
Files 995 997 +2
Lines 52884 52993 +109
Branches 6721 6743 +22
==========================================
+ Hits 40636 40674 +38
- Misses 12023 12094 +71
Partials 225 225
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.07% <37.71%> (-0.16%)` | :arrow_down: |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `85.00% <ø> (ø)` | |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.89% <33.33%> (-0.18%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.82% <50.00%> (-0.25%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.00% <60.86%> (+1.57%)` | :arrow_up: |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `88.88% <88.88%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [ddb5005...ebe574d](https://codecov.io/gh/apache/superset/pull/16119?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] github-actions[bot] commented on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-895369237
@kgabryje Ephemeral environment spinning up at http://54.187.27.254: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] henryyeh commented on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
henryyeh commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-901482395
🏷️ 2021.31
--
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (754bdc5) into [master](https://codecov.io/gh/apache/superset/commit/4df3672baad2791bef57f0348ccc0eaa8d09222e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4df3672) will **decrease** coverage by `0.22%`.
> The diff coverage is `76.47%`.
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.83% 76.60% -0.23%
==========================================
Files 996 996
Lines 53060 53060
Branches 6766 6766
==========================================
- Hits 40766 40648 -118
- Misses 12066 12184 +118
Partials 228 228
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.61% <ø> (+0.04%)` | :arrow_up: |
| postgres | `81.59% <ø> (ø)` | |
| presto | `?` | |
| python | `81.72% <ø> (-0.44%)` | :arrow_down: |
| sqlite | `81.23% <ø> (ø)` | |
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/16119?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/config.py](https://codecov.io/gh/apache/superset/pull/16119/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.30% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.78% <40.00%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `45.07% <66.66%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `47.36% <100.00%> (ø)` | |
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `77.27% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `88.03% <0.00%> (-1.66%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/superset/pull/16119/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [4df3672...754bdc5](https://codecov.io/gh/apache/superset/pull/16119?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] codecov[bot] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (37e6a91) into [master](https://codecov.io/gh/apache/superset/commit/772da8de6353f01ea1c64037af9695d5f10b71e4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (772da8d) will **decrease** coverage by `0.30%`.
> The diff coverage is `37.16%`.
> :exclamation: Current head 37e6a91 differs from pull request most recent head 702bbf4. Consider uploading reports for the commit 702bbf4 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.84% 76.54% -0.31%
==========================================
Files 995 997 +2
Lines 52886 52995 +109
Branches 6721 6743 +22
==========================================
- Hits 40641 40563 -78
- Misses 12020 12207 +187
Partials 225 225
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| javascript | `71.07% <37.16%> (-0.16%)` | :arrow_down: |
| presto | `?` | |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `85.00% <ø> (ø)` | |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.82% <33.33%> (-0.25%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.89% <33.33%> (-0.18%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.00% <60.86%> (+1.57%)` | :arrow_up: |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `88.88% <88.88%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.49%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/apache/superset/pull/16119/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [772da8d...702bbf4](https://codecov.io/gh/apache/superset/pull/16119?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] github-actions[bot] commented on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-896694893
@kgabryje Ephemeral environment spinning up at http://35.160.108.204: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 a change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r685326902
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,234 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form
+ layout="vertical"
+ id="metrics-edit-popover"
+ data-test="metrics-edit-popover"
+ >
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ data-test="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Calculated column')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
+ <FormItem label={t('Column')}>
Review comment:
We decided to keep this name for now
--
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (36a6be8) into [master](https://codecov.io/gh/apache/superset/commit/9876c36f6ec9cdfc7ad74efdffefd5c77b645161?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9876c36) will **decrease** coverage by `0.21%`.
> The diff coverage is `78.94%`.
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.71% 76.49% -0.22%
==========================================
Files 997 997
Lines 53248 53248
Branches 6779 6779
==========================================
- Hits 40849 40733 -116
- Misses 12169 12285 +116
Partials 230 230
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.56% <ø> (-0.05%)` | :arrow_down: |
| postgres | `81.62% <ø> (ø)` | |
| presto | `?` | |
| python | `81.71% <ø> (-0.43%)` | :arrow_down: |
| sqlite | `81.23% <ø> (-0.05%)` | :arrow_down: |
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/16119?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/config.py](https://codecov.io/gh/apache/superset/pull/16119/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.30% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.21% <57.14%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `45.07% <66.66%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `46.42% <100.00%> (ø)` | |
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `77.27% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.49%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `88.04% <0.00%> (-1.66%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/superset/pull/16119/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [9876c36...36a6be8](https://codecov.io/gh/apache/superset/pull/16119?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] kgabryje commented on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-895367206
/testenv up FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=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] ktmud commented on a change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r688827667
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,223 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form layout="vertical" id="metrics-edit-popover">
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Saved expressions')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
Review comment:
`SIMPLE` makes sense in the context of metrics because it's a "simple" way of configuring adhoc metrics. But for columns, there is nothing to configure so the "simplicity" of selecting a calculated column and an original column is the same.
I wonder whether we can just display a popover without tabs but render "Columns" and "Calculated columns" as [options groups](https://ant.design/components/select/#components-select-demo-optgroup). Then add the two tabs "SIMPLE" and "Custom SQL" when we add support adhoc calculated columns.
--
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 merged pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
kgabryje merged pull request #16119:
URL: https://github.com/apache/superset/pull/16119
--
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 #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-900211870
@ktmud @junlincc @villebro I changed the ghost button texts to "Drop columns/metrics here or click" (and similar for singular version and columns only 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] codecov[bot] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d2f7622) into [master](https://codecov.io/gh/apache/superset/commit/4df3672baad2791bef57f0348ccc0eaa8d09222e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4df3672) will **decrease** coverage by `0.09%`.
> The diff coverage is `38.70%`.
> :exclamation: Current head d2f7622 differs from pull request most recent head 754bdc5. Consider uploading reports for the commit 754bdc5 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.83% 76.73% -0.10%
==========================================
Files 996 998 +2
Lines 53060 53177 +117
Branches 6766 6796 +30
==========================================
+ Hits 40766 40807 +41
- Misses 12066 12138 +72
- Partials 228 232 +4
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.01% <38.70%> (-0.17%)` | :arrow_down: |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `77.27% <ø> (ø)` | |
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/16119/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.30% <ø> (ø)` | |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.72% <40.00%> (-0.06%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.89% <50.00%> (-0.18%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.75% <62.06%> (+1.38%)` | :arrow_up: |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `83.33% <83.33%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [4df3672...754bdc5](https://codecov.io/gh/apache/superset/pull/16119?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] michael-s-molina commented on a change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r689585944
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,223 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
Review comment:
@kgabryje @rusackas This code is not using the new Select component. It's using the Ant Design directly. We shouldn't use the Ant Design component directly anymore because the behavior of the select is different, like sorting the selected options, pagination, and lazy loading. To customize the options of the new Select you can just pass the element in the `label` prop of the values.
These PRs contain an example of customized options:
https://github.com/apache/superset/pull/16200
https://github.com/apache/superset/pull/16252
I don't think this style should be a prop of the Select component because it's something specific to the customized label. We have many different customized labels throughout the application as you can see in the PRs above.
--
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 #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-895356702
CC @michellethomas
--
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`master@6ac4f4e`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `37.71%`.
> :exclamation: Current head bea2d00 differs from pull request most recent head dc0514d. Consider uploading reports for the commit dc0514d to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
=========================================
Coverage ? 76.75%
=========================================
Files ? 997
Lines ? 52993
Branches ? 6743
=========================================
Hits ? 40674
Misses ? 12094
Partials ? 225
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.07% <37.71%> (?)` | |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `85.00% <ø> (ø)` | |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.89% <33.33%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.82% <50.00%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.00% <60.86%> (ø)` | |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `88.88% <88.88%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [6ac4f4e...dc0514d](https://codecov.io/gh/apache/superset/pull/16119?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] github-actions[bot] commented on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-900274467
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (36a6be8) into [master](https://codecov.io/gh/apache/superset/commit/9876c36f6ec9cdfc7ad74efdffefd5c77b645161?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9876c36) will **decrease** coverage by `0.24%`.
> The diff coverage is `78.94%`.
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.71% 76.47% -0.25%
==========================================
Files 997 997
Lines 53248 53248
Branches 6779 6779
==========================================
- Hits 40849 40721 -128
- Misses 12169 12297 +128
Partials 230 230
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.62% <ø> (ø)` | |
| presto | `?` | |
| python | `81.67% <ø> (-0.47%)` | :arrow_down: |
| sqlite | `81.23% <ø> (-0.05%)` | :arrow_down: |
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/16119?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/config.py](https://codecov.io/gh/apache/superset/pull/16119/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.30% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.21% <57.14%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `45.07% <66.66%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `46.42% <100.00%> (ø)` | |
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `77.27% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `94.04% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [9 more](https://codecov.io/gh/apache/superset/pull/16119/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [9876c36...36a6be8](https://codecov.io/gh/apache/superset/pull/16119?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] codecov[bot] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1cbd14) into [master](https://codecov.io/gh/apache/superset/commit/6ac4f4ef2fbd7c717a41f33ae511a54f3e10f098?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6ac4f4e) will **decrease** coverage by `0.08%`.
> The diff coverage is `38.26%`.
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.61% 76.52% -0.09%
==========================================
Files 996 998 +2
Lines 52919 53029 +110
Branches 6721 6744 +23
==========================================
+ Hits 40544 40583 +39
- Misses 12150 12221 +71
Partials 225 225
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.07% <38.26%> (-0.16%)` | :arrow_down: |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `85.00% <ø> (ø)` | |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.89% <33.33%> (-0.18%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.82% <50.00%> (-0.25%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.68% <62.50%> (+2.25%)` | :arrow_up: |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `88.88% <88.88%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [6ac4f4e...f1cbd14](https://codecov.io/gh/apache/superset/pull/16119?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] michellethomas commented on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
michellethomas commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-895636485
Seems like a good improvement to me 👍 I tested this a bit and didn't see any issues but I only tested with a couple of datasources so I can look around a bit more later. cc: @ktmud you also looked around at drag and drop, not sure if you have any thoughts about this change.
--
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 #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-897119895
It depends on which direction we want to guide user to be on.
Here's my concern. With the + sign visual indication and the "Click or drop a metric/column here" placeholder, I am almost 100% sure first time users will click, instead of visit the left data panel first as their first interaction with this popover.
Once user start on one path, and find it sufficient enough, it's very likely they stick with the more familiar path and reluctant to try the other, which may slowdown the adoption of dnd.
The fact that DND offers a much fast time to chart, compared to click-to-select, I do want to intentionally guide user to start with this path, and promote this path.
I prefer to keep it simple"Drop a metric/column here" as the "+" is a loud enough call to action for clicking.
If we have to explicitly indicate again, "Click or drop a metric/column here" makes more sense to me, definitely not to use "drag" in the wording.
Also, recently we do identify some sever regressions in the control panel due to increasing complexity of the product by adding too many FF and having multiple environments. I can't emphasize enough that maintaining both and making sure both work well together and individually can be costly and messy.
I would push much harder keeping DND only and try to resolve all the existing concerns, bottlenecks by improving this one solution if I am working on a close source/design heavy product. However, Superset has its unique OSS DNA which should be preserved and respected.
To whomever approve and support this hybrid solution, please do assess the code, and test this PR throughout, and consider the risk, complexity for long term. @ktmud @villebro
--
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 a change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r689570051
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,223 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form layout="vertical" id="metrics-edit-popover">
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Saved expressions')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
Review comment:
Ping @michael-s-molina about proposal to add support for Option Groups to the Select 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 a change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r685326332
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,234 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form
+ layout="vertical"
+ id="metrics-edit-popover"
+ data-test="metrics-edit-popover"
+ >
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ data-test="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Calculated column')}>
Review comment:
We decided to postpone a decision about renaming
--
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 #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-896691654
/testenv up FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true FEATURE_ENABLE_DND_WITH_CLICK_UX=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] villebro commented on a change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r684422145
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,234 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form
+ layout="vertical"
+ id="metrics-edit-popover"
+ data-test="metrics-edit-popover"
+ >
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ data-test="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Calculated column')}>
Review comment:
To make this less ambiguous with forthcoming adhoc calculated columns/expressions, I'd perhaps call these "Saved expressions" (I'd also recommend renaming them in the dataset modal to "expressions").
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,234 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form
+ layout="vertical"
+ id="metrics-edit-popover"
+ data-test="metrics-edit-popover"
+ >
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ data-test="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Calculated column')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
+ <FormItem label={t('Column')}>
Review comment:
Not perhaps super important right now, but I'd prefer to call these tabs "Saved", "Column" and later "Custom SQL" when we add support for ad-hoc expressions. (This `FormItem` label I'd spontaneously call `Regular Column`, but am open to other suggestions)
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,234 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form
+ layout="vertical"
+ id="metrics-edit-popover"
+ data-test="metrics-edit-popover"
+ >
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ data-test="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Calculated column')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
+ <FormItem label={t('Column')}>
+ <Select
+ value={selectedSimpleColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onSimpleColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedSimpleColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', simpleColumns.length)}
+ >
+ {simpleColumns.map(simpleColumn => (
+ <Select.Option
+ value={simpleColumn.column_name}
+ filterBy={
+ simpleColumn.verbose_name || simpleColumn.column_name
+ }
+ key={simpleColumn.column_name}
+ >
+ <StyledColumnOption column={simpleColumn} showType />
+ </Select.Option>
+ ))}
+ </Select>
+ </FormItem>
+ </Tabs.TabPane>
+ </Tabs>
+ <div>
+ <Button
+ buttonSize="small"
+ onClick={onResetStateAndClose}
+ data-test="AdhocMetricEdit#cancel"
Review comment:
Copy pasta 🙂
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,234 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form
+ layout="vertical"
+ id="metrics-edit-popover"
+ data-test="metrics-edit-popover"
+ >
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ data-test="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Calculated column')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
+ <FormItem label={t('Column')}>
+ <Select
+ value={selectedSimpleColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onSimpleColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedSimpleColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', simpleColumns.length)}
+ >
+ {simpleColumns.map(simpleColumn => (
+ <Select.Option
+ value={simpleColumn.column_name}
+ filterBy={
+ simpleColumn.verbose_name || simpleColumn.column_name
+ }
+ key={simpleColumn.column_name}
+ >
+ <StyledColumnOption column={simpleColumn} showType />
+ </Select.Option>
+ ))}
+ </Select>
+ </FormItem>
+ </Tabs.TabPane>
+ </Tabs>
+ <div>
+ <Button
+ buttonSize="small"
+ onClick={onResetStateAndClose}
+ data-test="AdhocMetricEdit#cancel"
+ cta
+ >
+ {t('Close')}
+ </Button>
+ <Button
+ disabled={!stateIsValid}
+ buttonStyle={
+ hasUnsavedChanges && stateIsValid ? 'primary' : 'default'
+ }
+ buttonSize="small"
+ data-test="AdhocMetricEdit#save"
Review comment:
same here
--
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 change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r688827667
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,223 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form layout="vertical" id="metrics-edit-popover">
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Saved expressions')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
Review comment:
`SIMPLE` makes sense in the context of metrics because it's a "simple" ways of configuring adhoc metrics. But for columns, there is nothing to configure and the "simplicity" of selecting a calculated column and an original column is the same.
I wonder whether we can just display a popover without tabs but render "Columns" and "Calculated columns" as [options groups](https://ant.design/components/select/#components-select-demo-optgroup). Then add the two tabs "SIMPLE" and "Custom SQL" when we support adhoc calcualted columns.
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,223 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form layout="vertical" id="metrics-edit-popover">
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Saved expressions')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
Review comment:
`SIMPLE` makes sense in the context of metrics because it's a "simple" ways of configuring adhoc metrics. But for columns, there is nothing to configure and the "simplicity" of selecting a calculated column and an original column is the same.
I wonder whether we can just display a popover without tabs but render "Columns" and "Calculated columns" as [options groups](https://ant.design/components/select/#components-select-demo-optgroup). Then add the two tabs "SIMPLE" and "Custom SQL" when we support adhoc calculated columns.
--
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 pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-896202620
Didn't have time to look at the code but this change is a big + from me!
We might want to change the copy on the ghost button as well. E.g. "Click or drag a metric/column here to add" like used in [SIP-62](https://github.com/apache/superset/issues/14114).
--
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (37e6a91) into [master](https://codecov.io/gh/apache/superset/commit/772da8de6353f01ea1c64037af9695d5f10b71e4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (772da8d) will **decrease** coverage by `0.30%`.
> The diff coverage is `37.16%`.
> :exclamation: Current head 37e6a91 differs from pull request most recent head 22152ce. Consider uploading reports for the commit 22152ce to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.84% 76.54% -0.31%
==========================================
Files 995 997 +2
Lines 52886 52995 +109
Branches 6721 6743 +22
==========================================
- Hits 40641 40563 -78
- Misses 12020 12207 +187
Partials 225 225
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| javascript | `71.07% <37.16%> (-0.16%)` | :arrow_down: |
| presto | `?` | |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `85.00% <ø> (ø)` | |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.82% <33.33%> (-0.25%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.89% <33.33%> (-0.18%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.00% <60.86%> (+1.57%)` | :arrow_up: |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `88.88% <88.88%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.49%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/apache/superset/pull/16119/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [772da8d...22152ce](https://codecov.io/gh/apache/superset/pull/16119?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] ktmud commented on a change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r688827667
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,223 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form layout="vertical" id="metrics-edit-popover">
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Saved expressions')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
Review comment:
`SIMPLE` makes sense in the context of metrics because it's a "simple" way of configuring adhoc metrics. But for columns, there is nothing to configure so the "simplicity" of selecting a calculated column and an original column is the same.
I wonder whether we can just display a popover without tabs but render "Columns" and "Calculated columns" as [options groups](https://ant.design/components/select/#components-select-demo-optgroup). Then add the two tabs "SIMPLE" and "Custom SQL" when we support adhoc calculated columns.
--
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (307fc7b) into [master](https://codecov.io/gh/apache/superset/commit/9876c36f6ec9cdfc7ad74efdffefd5c77b645161?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9876c36) will **decrease** coverage by `0.21%`.
> The diff coverage is `78.94%`.
> :exclamation: Current head 307fc7b differs from pull request most recent head 36a6be8. Consider uploading reports for the commit 36a6be8 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.71% 76.49% -0.22%
==========================================
Files 997 997
Lines 53248 53248
Branches 6779 6779
==========================================
- Hits 40849 40733 -116
- Misses 12169 12285 +116
Partials 230 230
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.56% <ø> (-0.05%)` | :arrow_down: |
| postgres | `81.63% <ø> (+<0.01%)` | :arrow_up: |
| presto | `?` | |
| python | `81.71% <ø> (-0.43%)` | :arrow_down: |
| sqlite | `81.23% <ø> (-0.05%)` | :arrow_down: |
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/16119?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/config.py](https://codecov.io/gh/apache/superset/pull/16119/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.30% <ø> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.21% <57.14%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `45.07% <66.66%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `46.42% <100.00%> (ø)` | |
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `77.27% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.49%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `88.04% <0.00%> (-1.66%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/superset/pull/16119/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [9876c36...36a6be8](https://codecov.io/gh/apache/superset/pull/16119?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] ktmud commented on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-897190375
Maybe promote DnD before click: "Drop a column/metric here or click"?
--
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 edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894483094
❤️ it as it's solid good work! as an OSS project, inclusiveness, flexibility and extensibility are always more important than chasing after perfect UX for a certain group of user regardless its size. :) I will help testing
--
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bea2d00) into [master](https://codecov.io/gh/apache/superset/commit/6ac4f4ef2fbd7c717a41f33ae511a54f3e10f098?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6ac4f4e) will **increase** coverage by `0.13%`.
> The diff coverage is `37.71%`.
> :exclamation: Current head bea2d00 differs from pull request most recent head f1cbd14. Consider uploading reports for the commit f1cbd14 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
+ Coverage 76.61% 76.75% +0.13%
==========================================
Files 996 997 +1
Lines 52919 52993 +74
Branches 6721 6743 +22
==========================================
+ Hits 40544 40674 +130
+ Misses 12150 12094 -56
Partials 225 225
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.07% <37.71%> (-0.16%)` | :arrow_down: |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `85.00% <ø> (ø)` | |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.89% <33.33%> (-0.18%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.82% <50.00%> (-0.25%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.00% <60.86%> (+1.57%)` | :arrow_up: |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `88.88% <88.88%> (ø)` | |
| [superset/charts/api.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `85.71% <0.00%> (-0.12%)` | :arrow_down: |
| [superset/dashboards/api.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9hcGkucHk=) | `92.95% <0.00%> (-0.08%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/16119/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% <0.00%> (-0.03%)` | :arrow_down: |
| [superset/utils/profiler.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvdXRpbHMvcHJvZmlsZXIucHk=) | | |
| ... and [12 more](https://codecov.io/gh/apache/superset/pull/16119/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [6ac4f4e...f1cbd14](https://codecov.io/gh/apache/superset/pull/16119?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] ktmud commented on a change in pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #16119:
URL: https://github.com/apache/superset/pull/16119#discussion_r688827667
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
##########
@@ -0,0 +1,223 @@
+/**
+ * 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.
+ */
+/* eslint-disable camelcase */
+import React, { useCallback, useMemo, useState } from 'react';
+import Tabs from 'src/components/Tabs';
+import Button from 'src/components/Button';
+import { NativeSelect as Select } from 'src/components/Select';
+import { t, styled } from '@superset-ui/core';
+
+import { Form, FormItem } from 'src/components/Form';
+import { StyledColumnOption } from 'src/explore/components/optionRenderers';
+import { ColumnMeta } from '@superset-ui/chart-controls';
+
+const StyledSelect = styled(Select)`
+ .metric-option {
+ & > svg {
+ min-width: ${({ theme }) => `${theme.gridUnit * 4}px`};
+ }
+ & > .option-label {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ }
+ }
+`;
+
+interface ColumnSelectPopoverProps {
+ columns: ColumnMeta[];
+ editedColumn?: ColumnMeta;
+ onChange: (column: ColumnMeta) => void;
+ onClose: () => void;
+}
+
+const ColumnSelectPopover = ({
+ columns,
+ editedColumn,
+ onChange,
+ onClose,
+}: ColumnSelectPopoverProps) => {
+ const [
+ initialCalculatedColumn,
+ initialSimpleColumn,
+ ] = editedColumn?.expression
+ ? [editedColumn, undefined]
+ : [undefined, editedColumn];
+ const [selectedCalculatedColumn, setSelectedCalculatedColumn] = useState(
+ initialCalculatedColumn,
+ );
+ const [selectedSimpleColumn, setSelectedSimpleColumn] = useState(
+ initialSimpleColumn,
+ );
+
+ const [calculatedColumns, simpleColumns] = useMemo(
+ () =>
+ columns?.reduce(
+ (acc: [ColumnMeta[], ColumnMeta[]], column: ColumnMeta) => {
+ if (column.expression) {
+ acc[0].push(column);
+ } else {
+ acc[1].push(column);
+ }
+ return acc;
+ },
+ [[], []],
+ ),
+ [columns],
+ );
+
+ const onCalculatedColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = calculatedColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(selectedColumn);
+ setSelectedSimpleColumn(undefined);
+ },
+ [calculatedColumns],
+ );
+
+ const onSimpleColumnChange = useCallback(
+ selectedColumnName => {
+ const selectedColumn = simpleColumns.find(
+ col => col.column_name === selectedColumnName,
+ );
+ setSelectedCalculatedColumn(undefined);
+ setSelectedSimpleColumn(selectedColumn);
+ },
+ [simpleColumns],
+ );
+
+ const defaultActiveTabKey =
+ initialSimpleColumn || calculatedColumns.length === 0 ? 'simple' : 'saved';
+
+ const onSave = useCallback(() => {
+ const selectedColumn = selectedCalculatedColumn || selectedSimpleColumn;
+ if (!selectedColumn) {
+ return;
+ }
+ onChange(selectedColumn);
+ onClose();
+ }, [onChange, onClose, selectedCalculatedColumn, selectedSimpleColumn]);
+
+ const onResetStateAndClose = useCallback(() => {
+ setSelectedCalculatedColumn(initialCalculatedColumn);
+ setSelectedSimpleColumn(initialSimpleColumn);
+ onClose();
+ }, [initialCalculatedColumn, initialSimpleColumn, onClose]);
+
+ const stateIsValid = selectedCalculatedColumn || selectedSimpleColumn;
+ const hasUnsavedChanges =
+ selectedCalculatedColumn?.column_name !==
+ initialCalculatedColumn?.column_name ||
+ selectedSimpleColumn?.column_name !== initialSimpleColumn?.column_name;
+
+ const filterOption = useCallback(
+ (input, option) =>
+ option?.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0,
+ [],
+ );
+
+ const getPopupContainer = useCallback(
+ (triggerNode: any) => triggerNode.parentNode,
+ [],
+ );
+
+ return (
+ <Form layout="vertical" id="metrics-edit-popover">
+ <Tabs
+ id="adhoc-metric-edit-tabs"
+ defaultActiveKey={defaultActiveTabKey}
+ className="adhoc-metric-edit-tabs"
+ allowOverflow
+ >
+ <Tabs.TabPane key="saved" tab={t('Saved')}>
+ <FormItem label={t('Saved expressions')}>
+ <StyledSelect
+ value={selectedCalculatedColumn?.column_name}
+ getPopupContainer={getPopupContainer}
+ onChange={onCalculatedColumnChange}
+ allowClear
+ showSearch
+ autoFocus={!selectedCalculatedColumn}
+ filterOption={filterOption}
+ placeholder={t('%s column(s)', calculatedColumns.length)}
+ >
+ {calculatedColumns.map(calculatedColumn => (
+ <Select.Option
+ value={calculatedColumn.column_name}
+ filterBy={
+ calculatedColumn.verbose_name ||
+ calculatedColumn.column_name
+ }
+ key={calculatedColumn.column_name}
+ >
+ <StyledColumnOption column={calculatedColumn} showType />
+ </Select.Option>
+ ))}
+ </StyledSelect>
+ </FormItem>
+ </Tabs.TabPane>
+ <Tabs.TabPane key="simple" tab={t('Simple')}>
Review comment:
`SIMPLE` makes sense in the context of metrics because it's a "simple" way of configuring adhoc metrics. But for columns, there is nothing to configure so the "simplicity" of selecting a calculated column and an original column is the same.
I wonder whether we can just display a popover without tabs but render "Columns" and "Calculated columns" as [options groups](https://ant.design/components/select/#components-select-demo-optgroup). Then add the two tabs "SIMPLE" and "Custom SQL" when we add support for adhoc calculated columns.
--
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] mdeshmu commented on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
mdeshmu commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-927111982
💯 I really appreciate that this is being added.
--
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 #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894483094
❤️ it as it's solid good work! as an OSS project, inclusiveness, flexibility and extensibility are always more important than chasing after perfect UX for a certain group of user regardless its size. :)
--
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] edited a comment on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-894369549
# [Codecov](https://codecov.io/gh/apache/superset/pull/16119?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 [#16119](https://codecov.io/gh/apache/superset/pull/16119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1cbd14) into [master](https://codecov.io/gh/apache/superset/commit/4df3672baad2791bef57f0348ccc0eaa8d09222e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4df3672) will **decrease** coverage by `0.30%`.
> The diff coverage is `38.26%`.
> :exclamation: Current head f1cbd14 differs from pull request most recent head 754bdc5. Consider uploading reports for the commit 754bdc5 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16119?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 #16119 +/- ##
==========================================
- Coverage 76.83% 76.52% -0.31%
==========================================
Files 996 998 +2
Lines 53060 53029 -31
Branches 6766 6744 -22
==========================================
- Hits 40766 40583 -183
- Misses 12066 12221 +155
+ Partials 228 225 -3
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| javascript | `71.07% <38.26%> (-0.10%)` | :arrow_down: |
| presto | `?` | |
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/16119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...controls/DndColumnSelectControl/DndSelectLabel.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZFNlbGVjdExhYmVsLnRzeA==) | `85.00% <ø> (+7.72%)` | :arrow_up: |
| [...ols/DndColumnSelectControl/ColumnSelectPopover.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXIudHN4) | `15.15% <15.15%> (ø)` | |
| [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.89% <33.33%> (+0.11%)` | :arrow_up: |
| [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `44.82% <50.00%> (-0.25%)` | :arrow_down: |
| [...ontrols/DndColumnSelectControl/DndColumnSelect.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZENvbHVtblNlbGVjdC50c3g=) | `48.68% <62.50%> (+1.31%)` | :arrow_up: |
| [...ColumnSelectControl/ColumnSelectPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0NvbHVtblNlbGVjdFBvcG92ZXJUcmlnZ2VyLnRzeA==) | `88.88% <88.88%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16119/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [48 more](https://codecov.io/gh/apache/superset/pull/16119/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16119?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/16119?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 [4df3672...754bdc5](https://codecov.io/gh/apache/superset/pull/16119?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] villebro commented on pull request #16119: feat(explore): make dnd controls clickable
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #16119:
URL: https://github.com/apache/superset/pull/16119#issuecomment-896813602
I would prefer "Click or drag a metric/column here" because:
- "to add" feels obvious in this context (I'm not sure what else the user could expect to happen by clicking or dragging a column/metric into the control)
- The string is already long, and in some other language it is more likely to overflow if we add more words.
--
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