You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/01/27 03:47:40 UTC
[GitHub] [superset] ad-m opened a new pull request #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
ad-m opened a new pull request #18188:
URL: https://github.com/apache/superset/pull/18188
### SUMMARY
Upgraded ControlHeader to TypeScript to apply direction outlined in #18100 .
Added storybook to visualize that component.
Upgraded to FC.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
Storybook can be used to verify those changes.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: see #18100
- [x] Required feature flags: no
- [x] Changes UI: no
- [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)): no
- [x] Migration is atomic, supports rollback & is backwards-compatible: N/A
- [x] Confirm DB migration upgrade and downgrade tested: N/A
- [x] Runtime estimates and downtime expectations provided: N/A
- [x] Introduces new feature or API: no
- [x] Removes existing feature or API: no
@kgabryje , @mik-laj You might be interested in this PR!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ad-m commented on a change in pull request #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
Posted by GitBox <gi...@apache.org>.
ad-m commented on a change in pull request #18188:
URL: https://github.com/apache/superset/pull/18188#discussion_r793904111
##########
File path: superset-frontend/src/explore/components/ControlHeader.tsx
##########
@@ -0,0 +1,154 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FC, ReactNode } from 'react';
+import { t, css, useTheme, JsonObject } from '@superset-ui/core';
+import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
+import { Tooltip } from 'src/components/Tooltip';
+import { FormLabel } from 'src/components/Form';
+import Icons from 'src/components/Icons';
+
+export type ControlHeaderProps = {
+ name?: string;
+ label?: ReactNode;
+ description?: ReactNode;
+ validationErrors?: JsonObject | null;
Review comment:
I get inspired by https://github.com/apache/superset/blob/40b88f04f6d360d66fcab7206e2e481d71ddf884/superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm/index.tsx#L69 . It was not fully clear to me what the format is in it and I had a hard time tracing the format of this parameter from using this component. However, looking again at this element, especially that what was previously (optional `string[]`) before, I changed the format. Thanks for pointing this out!
--
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 #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18188:
URL: https://github.com/apache/superset/pull/18188#issuecomment-1023447085
@geido Ephemeral environment spinning up at http://34.215.33.208: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] codecov[bot] edited a comment on pull request #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18188:
URL: https://github.com/apache/superset/pull/18188#issuecomment-1022838727
# [Codecov](https://codecov.io/gh/apache/superset/pull/18188?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 [#18188](https://codecov.io/gh/apache/superset/pull/18188?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (199599c) into [master](https://codecov.io/gh/apache/superset/commit/e6bb0fc7ad61dc314156b283dcfbbd991ddaff9a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6bb0fc) will **increase** coverage by `0.00%`.
> The diff coverage is `82.60%`.
> :exclamation: Current head 199599c differs from pull request most recent head e543e33. Consider uploading reports for the commit e543e33 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18188?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 #18188 +/- ##
=======================================
Coverage 66.04% 66.04%
=======================================
Files 1591 1591
Lines 62415 62417 +2
Branches 6285 6288 +3
=======================================
+ Hits 41224 41226 +2
Misses 19569 19569
Partials 1622 1622
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `50.86% <82.60%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/18188?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...-frontend/src/explore/components/ControlHeader.tsx](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sSGVhZGVyLnRzeA==) | `82.60% <82.60%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18188?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/18188?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 [e6bb0fc...e543e33](https://codecov.io/gh/apache/superset/pull/18188?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 #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18188:
URL: https://github.com/apache/superset/pull/18188#issuecomment-1026057830
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 #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18188:
URL: https://github.com/apache/superset/pull/18188#issuecomment-1022838727
# [Codecov](https://codecov.io/gh/apache/superset/pull/18188?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 [#18188](https://codecov.io/gh/apache/superset/pull/18188?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e543e33) into [master](https://codecov.io/gh/apache/superset/commit/e6bb0fc7ad61dc314156b283dcfbbd991ddaff9a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6bb0fc) will **increase** coverage by `0.00%`.
> The diff coverage is `82.60%`.
[](https://codecov.io/gh/apache/superset/pull/18188?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 #18188 +/- ##
=======================================
Coverage 66.04% 66.04%
=======================================
Files 1591 1591
Lines 62415 62421 +6
Branches 6285 6289 +4
=======================================
+ Hits 41224 41229 +5
- Misses 19569 19570 +1
Partials 1622 1622
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `50.86% <82.60%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/18188?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...-frontend/src/explore/components/ControlHeader.tsx](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sSGVhZGVyLnRzeA==) | `82.60% <82.60%> (ø)` | |
| [...ashboard/components/UndoRedoKeyListeners/index.jsx](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1VuZG9SZWRvS2V5TGlzdGVuZXJzL2luZGV4LmpzeA==) | `100.00% <0.00%> (ø)` | |
| [...lugin-chart-echarts/src/Timeseries/transformers.ts](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1lcnMudHM=) | `50.83% <0.00%> (ø)` | |
| [...chart-echarts/src/MixedTimeseries/controlPanel.tsx](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL2NvbnRyb2xQYW5lbC50c3g=) | `80.00% <0.00%> (ø)` | |
| [...hart-echarts/src/MixedTimeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL3RyYW5zZm9ybVByb3BzLnRz) | `0.00% <0.00%> (ø)` | |
| [...eFilters/FiltersConfigModal/FiltersConfigModal.tsx](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdNb2RhbC50c3g=) | `79.62% <0.00%> (ø)` | |
| [...l/AdhocFilterEditPopoverSimpleTabContent/index.tsx](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTaW1wbGVUYWJDb250ZW50L2luZGV4LnRzeA==) | `65.07% <0.00%> (ø)` | |
| [superset-frontend/src/utils/common.js](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2NvbW1vbi5qcw==) | `88.67% <0.00%> (+0.21%)` | :arrow_up: |
| [...uperset-frontend/src/explore/exploreUtils/index.js](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvZXhwbG9yZVV0aWxzL2luZGV4Lmpz) | `67.13% <0.00%> (+0.23%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18188?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/18188?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 [e6bb0fc...e543e33](https://codecov.io/gh/apache/superset/pull/18188?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] commented on pull request #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #18188:
URL: https://github.com/apache/superset/pull/18188#issuecomment-1022838727
# [Codecov](https://codecov.io/gh/apache/superset/pull/18188?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 [#18188](https://codecov.io/gh/apache/superset/pull/18188?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (199599c) into [master](https://codecov.io/gh/apache/superset/commit/e6bb0fc7ad61dc314156b283dcfbbd991ddaff9a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6bb0fc) will **increase** coverage by `0.00%`.
> The diff coverage is `82.60%`.
[](https://codecov.io/gh/apache/superset/pull/18188?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 #18188 +/- ##
=======================================
Coverage 66.04% 66.04%
=======================================
Files 1591 1591
Lines 62415 62417 +2
Branches 6285 6288 +3
=======================================
+ Hits 41224 41226 +2
Misses 19569 19569
Partials 1622 1622
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `50.86% <82.60%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/18188?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...-frontend/src/explore/components/ControlHeader.tsx](https://codecov.io/gh/apache/superset/pull/18188/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sSGVhZGVyLnRzeA==) | `82.60% <82.60%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18188?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/18188?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 [e6bb0fc...199599c](https://codecov.io/gh/apache/superset/pull/18188?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] geido commented on pull request #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
Posted by GitBox <gi...@apache.org>.
geido commented on pull request #18188:
URL: https://github.com/apache/superset/pull/18188#issuecomment-1023445251
/testenv up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 merged pull request #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
Posted by GitBox <gi...@apache.org>.
etr2460 merged pull request #18188:
URL: https://github.com/apache/superset/pull/18188
--
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] etr2460 commented on a change in pull request #18188: refactor: upgrade ControlHeader to TSX & FC and add storybook
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #18188:
URL: https://github.com/apache/superset/pull/18188#discussion_r793801636
##########
File path: superset-frontend/src/explore/components/ControlHeader.stories.tsx
##########
@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import ControlHeader, { ControlHeaderProps } from './ControlHeader';
+
+export default {
+ title: 'ControlHeader',
+ component: ControlHeader,
+};
+
+const options: {
+ [key: string]: ControlHeaderProps;
+} = {
+ label: {
+ label: 'Control label',
+ },
+ warning: {
+ label: 'Control warning',
+ warning: 'Example of warning message',
+ },
+ error: {
+ label: 'Control error',
+ validationErrors: ['Something is wrong'],
+ },
+};
+
+export const ControlHeaderGallery = () => (
+ <>
+ {Object.entries(options).map(([name, props]) => (
+ <>
+ <h4>{name}</h4>
+ <ControlHeader {...props} />
+ </>
+ ))}
+ </>
+);
+
+export const InteractiveControlHeader = (props: ControlHeaderProps) => (
+ <>
+ <ControlHeader {...props} />
+ </>
Review comment:
no fragment needed
##########
File path: superset-frontend/src/explore/components/ControlHeader.stories.tsx
##########
@@ -0,0 +1,79 @@
+/**
Review comment:
love the stories, thanks!
##########
File path: superset-frontend/src/explore/components/ControlHeader.tsx
##########
@@ -0,0 +1,154 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FC, ReactNode } from 'react';
+import { t, css, useTheme, JsonObject } from '@superset-ui/core';
+import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
+import { Tooltip } from 'src/components/Tooltip';
+import { FormLabel } from 'src/components/Form';
+import Icons from 'src/components/Icons';
+
+export type ControlHeaderProps = {
+ name?: string;
+ label?: ReactNode;
+ description?: ReactNode;
+ validationErrors?: JsonObject | null;
Review comment:
validation errors was an array before, and now you've set it to a json object. Was that intended? seems like it can always be an `any[]`, and maybe even a `string[]`
##########
File path: superset-frontend/src/explore/components/ControlHeader.tsx
##########
@@ -0,0 +1,154 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FC, ReactNode } from 'react';
+import { t, css, useTheme, JsonObject } from '@superset-ui/core';
+import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
+import { Tooltip } from 'src/components/Tooltip';
+import { FormLabel } from 'src/components/Form';
+import Icons from 'src/components/Icons';
+
+export type ControlHeaderProps = {
+ name?: string;
+ label?: ReactNode;
+ description?: ReactNode;
+ validationErrors?: JsonObject | null;
+ renderTrigger?: boolean;
+ rightNode?: ReactNode;
+ leftNode?: ReactNode;
+ onClick?: () => {};
+ hovered?: boolean;
+ tooltipOnClick?: () => {};
Review comment:
```suggestion
tooltipOnClick?: () => void;
```
##########
File path: superset-frontend/src/explore/components/ControlHeader.tsx
##########
@@ -0,0 +1,154 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FC, ReactNode } from 'react';
+import { t, css, useTheme, JsonObject } from '@superset-ui/core';
+import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
+import { Tooltip } from 'src/components/Tooltip';
+import { FormLabel } from 'src/components/Form';
+import Icons from 'src/components/Icons';
+
+export type ControlHeaderProps = {
+ name?: string;
+ label?: ReactNode;
+ description?: ReactNode;
+ validationErrors?: JsonObject | null;
+ renderTrigger?: boolean;
+ rightNode?: ReactNode;
+ leftNode?: ReactNode;
+ onClick?: () => {};
Review comment:
```suggestion
onClick?: () => void;
```
I think this is what you want, unless onClick actually returns an empty object
##########
File path: superset-frontend/src/explore/components/ControlHeader.tsx
##########
@@ -0,0 +1,154 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FC, ReactNode } from 'react';
+import { t, css, useTheme, JsonObject } from '@superset-ui/core';
+import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
+import { Tooltip } from 'src/components/Tooltip';
+import { FormLabel } from 'src/components/Form';
+import Icons from 'src/components/Icons';
+
+export type ControlHeaderProps = {
+ name?: string;
+ label?: ReactNode;
+ description?: ReactNode;
+ validationErrors?: JsonObject | null;
+ renderTrigger?: boolean;
+ rightNode?: ReactNode;
+ leftNode?: ReactNode;
+ onClick?: () => {};
+ hovered?: boolean;
+ tooltipOnClick?: () => {};
+ warning?: string;
+ danger?: string;
+};
+
+const ControlHeader: FC<ControlHeaderProps> = ({
+ name,
+ label,
+ description,
+ validationErrors = [],
+ renderTrigger = false,
+ rightNode,
+ leftNode,
+ onClick,
+ hovered = false,
+ tooltipOnClick = () => {},
+ warning,
+ danger,
+}) => {
+ const { gridUnit, colors } = useTheme();
+
+ const renderOptionalIcons = () => {
+ if (!hovered) {
+ return null;
+ }
+
+ return (
+ <span
+ css={() => css`
+ position: absolute;
+ top: 50%;
+ right: 0;
+ padding-left: ${gridUnit}px;
+ transform: translate(100%, -50%);
+ white-space: nowrap;
+ `}
+ >
+ {description && (
+ <span>
+ <InfoTooltipWithTrigger
+ label={t('description')}
+ tooltip={description}
+ placement="top"
+ onClick={tooltipOnClick}
+ />{' '}
+ </span>
+ )}
+ {renderTrigger && (
+ <span>
+ <InfoTooltipWithTrigger
+ label={t('bolt')}
+ tooltip={t('Changing this control takes effect instantly')}
+ placement="top"
+ icon="bolt"
+ />{' '}
+ </span>
+ )}
+ </span>
+ );
+ };
+ if (!label) {
+ return null;
+ }
Review comment:
maybe this should be before the `renderOptionalIcons` function def so that we can return early?
--
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