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/02/03 14:24:45 UTC
[GitHub] [superset] simcha90 opened a new pull request #12918: feat(native-filters): hide dashboard header bu url parameter
simcha90 opened a new pull request #12918:
URL: https://github.com/apache/superset/pull/12918
### SUMMARY
- Hide dashboard header with edit button dependent on url parameter
- Add global constants for url params
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
https://user-images.githubusercontent.com/56388545/106760092-1917e780-663c-11eb-99cb-43574ad01b8d.mov
### TEST PLAN
- Test with param `hide_dashboard_header` and without it
- Test filter bar of native filters working as expected
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] simcha90 commented on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-776590533
@ktmud I can't reproduce UI bug that you sent, also Chrome also Firefox
----------------------------------------------------------------
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.
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] simcha90 commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573152532
##########
File path: superset/views/core.py
##########
@@ -400,9 +401,11 @@ def slice(self, slice_id: int) -> FlaskResponse: # pylint: disable=no-self-use
endpoint = "/superset/explore/?form_data={}".format(
parse.quote(json.dumps({"slice_id": slice_id}))
)
- param = utils.ReservedUrlParameters.STANDALONE.value
- if request.args.get(param) == "true":
- endpoint += f"&{param}=true"
+
+ is_standalone_mode = ReservedUrlParameters.is_standalone_mode()
+ if is_standalone_mode:
+ param = utils.ReservedUrlParameters.STANDALONE.value
+ endpoint += f"&{param}={is_standalone_mode}"
Review comment:
done
----------------------------------------------------------------
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.
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] amitmiran137 commented on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-773178658
@ktmud I agree that the use-case of standalone=false and hide_dashboard_hide=true does not exist
but there are 2 cases that do exist
1. standalone=true & hide_dashboard_hide=false
2. standalone =true & hide_dashboard_hide=true
I can agree that by just setting hide_dashboard_hide=true we can enable standalone but not sure we must enforce it
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-773165957
----------------------------------------------------------------
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.
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] simcha90 commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573147684
##########
File path: superset-frontend/src/dashboard/util/getDashboardUrl.ts
##########
@@ -0,0 +1,75 @@
+/**
+ * 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 { URL_PARAMS } from 'src/constants';
+import serializeActiveFilterValues from './serializeActiveFilterValues';
+
+export default function getDashboardUrl(
+ pathname: string,
+ filters = {},
+ hash = '',
+ standalone?: number | null,
+) {
+ const newSearchParams = new URLSearchParams();
+
+ // convert flattened { [id_column]: values } object
+ // to nested filter object
+ newSearchParams.set(
+ URL_PARAMS.preselectFilters,
+ JSON.stringify(serializeActiveFilterValues(filters)),
+ );
+
+ if (standalone) {
+ newSearchParams.set(URL_PARAMS.standalone, standalone.toString());
+ }
+
+ const hashSection = hash ? `#${hash}` : '';
+
+ return `${pathname}?${newSearchParams.toString()}${hashSection}`;
+}
+
+export type UrlParamType = 'string' | 'number' | 'boolean';
+export function getUrlParam(paramName: string, type: 'string'): string;
+export function getUrlParam(paramName: string, type: 'number'): number;
+export function getUrlParam(paramName: string, type: 'boolean'): boolean;
+export function getUrlParam(paramName: string, type: UrlParamType): unknown {
Review comment:
moved it here: `superset-frontend/src/utils/common.ts`
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573271172
##########
File path: superset-frontend/src/utils/common.ts
##########
@@ -44,7 +44,13 @@ export function getParamFromQuery(query, param) {
return null;
}
-export function storeQuery(query) {
+export function storeQuery(query: {
+ dbId: any;
+ title: any;
+ schema: any;
+ autorun: any;
+ sql: any;
+}) {
Review comment:
https://github.com/apache/superset/blob/9997abeab6a5016f07872e9741f2bfba1686895f/superset-frontend/src/SqlLab/components/ShareSqlLabQuery.jsx#L34-L41
Maybe make the typing consistent with here?
To avoid having to update typing for all other util functions, you may also create `getParamFromQuery` as its own file. I think in general we should welcome smaller util files so it's easier to maintain and search (to find a specific util function you can just search by file name instead of search file content).
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
# [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=h1) Report
> Merging [#12918](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=desc) (c02286f) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `0.36%`.
> The diff coverage is `58.69%`.
[](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12918 +/- ##
==========================================
+ Coverage 53.06% 53.42% +0.36%
==========================================
Files 489 490 +1
Lines 17314 17326 +12
Branches 4482 4490 +8
==========================================
+ Hits 9187 9256 +69
+ Misses 8127 8070 -57
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `53.42% <58.69%> (+0.36%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rontend/src/components/ListView/CardSortSelect.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvQ2FyZFNvcnRTZWxlY3QudHN4) | `78.94% <ø> (ø)` | |
| [...ontend/src/components/ListViewCard/ImageLoader.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL0ltYWdlTG9hZGVyLnRzeA==) | `75.00% <0.00%> (ø)` | |
| [...set-frontend/src/components/URLShortLinkButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rQnV0dG9uLmpzeA==) | `100.00% <ø> (ø)` | |
| [...rset-frontend/src/components/URLShortLinkModal.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rTW9kYWwudHN4) | `77.77% <ø> (ø)` | |
| [...src/dashboard/components/HeaderActionsDropdown.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlckFjdGlvbnNEcm9wZG93bi5qc3g=) | `68.49% <ø> (+0.92%)` | :arrow_up: |
| [...end/src/dashboard/components/StickyVerticalBar.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1N0aWNreVZlcnRpY2FsQmFyLnRzeA==) | `100.00% <ø> (ø)` | |
| [...rontend/src/explore/components/EmbedCodeButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FbWJlZENvZGVCdXR0b24uanN4) | `80.76% <ø> (ø)` | |
| [...ntend/src/explore/components/ExploreChartPanel.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ2hhcnRQYW5lbC5qc3g=) | `83.58% <ø> (ø)` | |
| [superset-frontend/src/utils/common.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2NvbW1vbi5qcw==) | `18.18% <ø> (-0.66%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `79.81% <ø> (+5.50%)` | :arrow_up: |
| ... and [25 more](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=footer). Last update [3e0681b...c02286f](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
# [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=h1) Report
> Merging [#12918](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=desc) (2054d66) into [master](https://codecov.io/gh/apache/superset/commit/fd2d87340b9741a722b3e7e9a391fe6278d7b3e7?el=desc) (fd2d873) will **decrease** coverage by `16.14%`.
> The diff coverage is `92.00%`.
[](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12918 +/- ##
===========================================
- Coverage 66.98% 50.84% -16.15%
===========================================
Files 1026 477 -549
Lines 50351 17227 -33124
Branches 5189 4449 -740
===========================================
- Hits 33728 8759 -24969
+ Misses 16489 8468 -8021
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.84% <92.00%> (+0.04%)` | :arrow_up: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...src/dashboard/components/HeaderActionsDropdown.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlckFjdGlvbnNEcm9wZG93bi5qc3g=) | `67.56% <ø> (-2.71%)` | :arrow_down: |
| [...end/src/dashboard/components/StickyVerticalBar.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1N0aWNreVZlcnRpY2FsQmFyLnRzeA==) | `100.00% <ø> (ø)` | |
| [...rontend/src/explore/components/EmbedCodeButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FbWJlZENvZGVCdXR0b24uanN4) | `80.76% <ø> (ø)` | |
| [...set-frontend/src/dashboard/util/getDashboardUrl.ts](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZFVybC50cw==) | `69.23% <77.77%> (ø)` | |
| [superset-frontend/src/constants.ts](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbnN0YW50cy50cw==) | `100.00% <100.00%> (ø)` | |
| [...tend/src/dashboard/components/DashboardBuilder.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `86.74% <100.00%> (+0.84%)` | :arrow_up: |
| [...nd/src/explore/components/ExploreViewContainer.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlVmlld0NvbnRhaW5lci5qc3g=) | `78.65% <100.00%> (+1.41%)` | :arrow_up: |
| [superset-frontend/src/explore/exploreUtils.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvZXhwbG9yZVV0aWxzLmpz) | `58.28% <100.00%> (-12.27%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [884 more](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=footer). Last update [2f6d1ff...ac4ac8c](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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-io commented on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
# [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=h1) Report
> Merging [#12918](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=desc) (2054d66) into [master](https://codecov.io/gh/apache/superset/commit/fd2d87340b9741a722b3e7e9a391fe6278d7b3e7?el=desc) (fd2d873) will **decrease** coverage by `15.51%`.
> The diff coverage is `92.00%`.
[](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12918 +/- ##
===========================================
- Coverage 66.98% 51.47% -15.52%
===========================================
Files 1026 438 -588
Lines 50351 14745 -35606
Branches 5189 3927 -1262
===========================================
- Hits 33728 7590 -26138
+ Misses 16489 7155 -9334
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `51.47% <92.00%> (+0.67%)` | :arrow_up: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...src/dashboard/components/HeaderActionsDropdown.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlckFjdGlvbnNEcm9wZG93bi5qc3g=) | `60.81% <ø> (-9.46%)` | :arrow_down: |
| [...end/src/dashboard/components/StickyVerticalBar.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1N0aWNreVZlcnRpY2FsQmFyLnRzeA==) | `100.00% <ø> (ø)` | |
| [...rontend/src/explore/components/EmbedCodeButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FbWJlZENvZGVCdXR0b24uanN4) | `80.76% <ø> (ø)` | |
| [...set-frontend/src/dashboard/util/getDashboardUrl.ts](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZFVybC50cw==) | `69.23% <77.77%> (ø)` | |
| [superset-frontend/src/constants.ts](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbnN0YW50cy50cw==) | `100.00% <100.00%> (ø)` | |
| [...tend/src/dashboard/components/DashboardBuilder.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `86.74% <100.00%> (+0.84%)` | :arrow_up: |
| [...nd/src/explore/components/ExploreViewContainer.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlVmlld0NvbnRhaW5lci5qc3g=) | `78.65% <100.00%> (+1.41%)` | :arrow_up: |
| [superset-frontend/src/explore/exploreUtils.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvZXhwbG9yZVV0aWxzLmpz) | `58.28% <100.00%> (-12.27%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [908 more](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=footer). Last update [2f6d1ff...ac4ac8c](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-773502496
How about we introduce two "standalone modes"?
- `standelone=true` or `standlone=1`: hide navs only
- `standalone=2`: hide navs and title
Let's not give users the choice to hide just the dashboard title. If the request comes back up with a reasonable business need to support it, then we can add it as `standlone=3`.
----------------------------------------------------------------
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.
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] simcha90 removed a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 removed a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-776590533
@ktmud I can't reproduce UI bug that you sent, also Chrome also Firefox
----------------------------------------------------------------
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.
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] simcha90 commented on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-773118838
> Why not call the parameter `standalone`, too, just to be consistent with charts?
`standalone` - hides only top menu bar
`hide_dashboard_hide` - hides only header with edit button
I think it dependent on user needs we need give both options to manage dashboard view
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
# [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=h1) Report
> Merging [#12918](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=desc) (2054d66) into [master](https://codecov.io/gh/apache/superset/commit/fd2d87340b9741a722b3e7e9a391fe6278d7b3e7?el=desc) (fd2d873) will **decrease** coverage by `16.51%`.
> The diff coverage is `92.00%`.
[](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12918 +/- ##
===========================================
- Coverage 66.98% 50.47% -16.52%
===========================================
Files 1026 477 -549
Lines 50351 17227 -33124
Branches 5189 4449 -740
===========================================
- Hits 33728 8695 -25033
+ Misses 16489 8532 -7957
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.47% <92.00%> (-0.33%)` | :arrow_down: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...src/dashboard/components/HeaderActionsDropdown.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlckFjdGlvbnNEcm9wZG93bi5qc3g=) | `67.56% <ø> (-2.71%)` | :arrow_down: |
| [...end/src/dashboard/components/StickyVerticalBar.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1N0aWNreVZlcnRpY2FsQmFyLnRzeA==) | `100.00% <ø> (ø)` | |
| [...rontend/src/explore/components/EmbedCodeButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FbWJlZENvZGVCdXR0b24uanN4) | `80.76% <ø> (ø)` | |
| [...set-frontend/src/dashboard/util/getDashboardUrl.ts](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZFVybC50cw==) | `69.23% <77.77%> (ø)` | |
| [superset-frontend/src/constants.ts](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbnN0YW50cy50cw==) | `100.00% <100.00%> (ø)` | |
| [...tend/src/dashboard/components/DashboardBuilder.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `86.74% <100.00%> (+0.84%)` | :arrow_up: |
| [...nd/src/explore/components/ExploreViewContainer.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlVmlld0NvbnRhaW5lci5qc3g=) | `78.65% <100.00%> (+1.41%)` | :arrow_up: |
| [superset-frontend/src/explore/exploreUtils.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvZXhwbG9yZVV0aWxzLmpz) | `58.28% <100.00%> (-12.27%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [884 more](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=footer). Last update [2f6d1ff...ac4ac8c](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] simcha90 commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573148669
##########
File path: superset-frontend/src/dashboard/util/getDashboardUrl.ts
##########
@@ -0,0 +1,75 @@
+/**
+ * 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 { URL_PARAMS } from 'src/constants';
+import serializeActiveFilterValues from './serializeActiveFilterValues';
+
+export default function getDashboardUrl(
+ pathname: string,
+ filters = {},
+ hash = '',
+ standalone?: number | null,
+) {
+ const newSearchParams = new URLSearchParams();
+
+ // convert flattened { [id_column]: values } object
+ // to nested filter object
+ newSearchParams.set(
+ URL_PARAMS.preselectFilters,
+ JSON.stringify(serializeActiveFilterValues(filters)),
+ );
+
+ if (standalone) {
+ newSearchParams.set(URL_PARAMS.standalone, standalone.toString());
+ }
+
+ const hashSection = hash ? `#${hash}` : '';
+
+ return `${pathname}?${newSearchParams.toString()}${hashSection}`;
+}
+
+export type UrlParamType = 'string' | 'number' | 'boolean';
+export function getUrlParam(paramName: string, type: 'string'): string;
+export function getUrlParam(paramName: string, type: 'number'): number;
+export function getUrlParam(paramName: string, type: 'boolean'): boolean;
+export function getUrlParam(paramName: string, type: UrlParamType): unknown {
+ const urlParam = new URLSearchParams(window.location.search.substring(1)).get(
Review comment:
done
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r574448146
##########
File path: superset-frontend/spec/javascripts/explore/utils_spec.jsx
##########
@@ -32,6 +32,7 @@ import {
} from 'src/explore/dateFilterUtils';
import * as hostNamesConfig from 'src/utils/hostNamesConfig';
import { getChartMetadataRegistry } from '@superset-ui/core';
+import { DashboardStandaloneMode } from '../../../src/dashboard/util/constants';
Review comment:
```suggestion
import { DashboardStandaloneMode } from 'src/dashboard/util/constants';
```
##########
File path: superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js
##########
@@ -18,28 +18,51 @@
*/
import getDashboardUrl from 'src/dashboard/util/getDashboardUrl';
import { DASHBOARD_FILTER_SCOPE_GLOBAL } from 'src/dashboard/reducers/dashboardFilters';
+import { DashboardStandaloneMode } from '../../../../src/dashboard/util/constants';
describe('getChartIdsFromLayout', () => {
+ const filters = {
+ '35_key': {
+ values: ['value'],
+ scope: DASHBOARD_FILTER_SCOPE_GLOBAL,
+ },
+ };
+
+ const globalLocation = window.location;
+ afterEach(() => {
+ window.location = globalLocation;
+ });
+
it('should encode filters', () => {
- const filters = {
- '35_key': {
- values: ['value'],
- scope: DASHBOARD_FILTER_SCOPE_GLOBAL,
- },
- };
const url = getDashboardUrl('path', filters);
expect(url).toBe(
'path?preselect_filters=%7B%2235%22%3A%7B%22key%22%3A%5B%22value%22%5D%7D%7D',
);
+ });
+ it('should encode filters with hash', () => {
const urlWithHash = getDashboardUrl('path', filters, 'iamhashtag');
expect(urlWithHash).toBe(
'path?preselect_filters=%7B%2235%22%3A%7B%22key%22%3A%5B%22value%22%5D%7D%7D#iamhashtag',
);
+ });
- const urlWithStandalone = getDashboardUrl('path', filters, '', true);
+ it('should encode filters with standalone', () => {
+ const urlWithStandalone = getDashboardUrl(
+ 'path',
+ filters,
+ '',
+ DashboardStandaloneMode.HIDE_NAV,
+ );
expect(urlWithStandalone).toBe(
- 'path?preselect_filters=%7B%2235%22%3A%7B%22key%22%3A%5B%22value%22%5D%7D%7D&standalone=true',
+ `path?preselect_filters=%7B%2235%22%3A%7B%22key%22%3A%5B%22value%22%5D%7D%7D&standalone=${DashboardStandaloneMode.HIDE_NAV}`,
+ );
+ });
+
+ it('should encode filters with missed standalone', () => {
Review comment:
```suggestion
it('should encode filters with missing standalone', () => {
```
##########
File path: superset-frontend/spec/javascripts/explore/components/EmbedCodeButton_spec.jsx
##########
@@ -26,7 +26,8 @@ import configureStore from 'redux-mock-store';
import fetchMock from 'fetch-mock';
import EmbedCodeButton from 'src/explore/components/EmbedCodeButton';
import * as exploreUtils from 'src/explore/exploreUtils';
-import * as common from 'src/utils/common';
+import * as urlUtils from 'src/utils/urlUtils';
+import { DashboardStandaloneMode } from '../../../../src/dashboard/util/constants';
Review comment:
```suggestion
import { DashboardStandaloneMode } from 'src/dashboard/util/constants';
```
----------------------------------------------------------------
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.
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] simcha90 commented on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-776602060
@ktmud 👍 good catch about UI bug, fixed
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-776914276
cc @junlincc if you want to do a final round of QA & product sign-off.
----------------------------------------------------------------
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.
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] simcha90 commented on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-773118838
> Why not call the parameter `standalone`, too, just to be consistent with charts?
`standalone` - hides only top menu bar
`hide_dashboard_hide` - hides only header with edit button
I think it dependent on user needs we need give both options to manage dashboard view
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772935621
Why not call the parameter `standalone`, too, just to be consistent with charts?
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772889577
----------------------------------------------------------------
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.
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 merged pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #12918:
URL: https://github.com/apache/superset/pull/12918
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573966622
##########
File path: superset-frontend/src/explore/exploreUtils.js
##########
@@ -99,8 +100,8 @@ export function getExploreLongUrl(
search[key] = extraSearch[key];
});
search.form_data = safeStringify(formData);
- if (endpointType === 'standalone') {
- search.standalone = 'true';
+ if (endpointType === URL_PARAMS.standalone) {
+ search.standalone = '1';
Review comment:
Could you use the constant `DashboardStandaloneMode` here as well?
Ditto in the spec files.
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772889577
thanks for the PR @simcha90 ! User will still be able to add filters after hiding header and edit button since the actions take place in the view mode. is this intentional?
@mihir174 I think this is a valid user need - sharing a dashboard to those who should not be editing it. this solution to me is a bit "hacky" though. any idea?
feature works as expected!
<img width="1787" alt="Screen Shot 2021-02-03 at 3 15 48 PM" src="https://user-images.githubusercontent.com/67837651/106821824-c4bc3a00-6632-11eb-8146-3993df429d52.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772889577
thanks for the PR @simcha90 ! User will still be able to add filters after hiding header and edit button since the actions take place in the view mode. is this intentional?
@mihir174 I think this is a valid user need - sharing a dashboard to those who should not be editing it. this solution to me is a bit "hacky" though. any idea?
from Product standpoint, this is a good start, if the code looks good, let's let it in and iterate on the the UX?
feature works as expected!
<img width="1787" alt="Screen Shot 2021-02-03 at 3 15 48 PM" src="https://user-images.githubusercontent.com/67837651/106821824-c4bc3a00-6632-11eb-8146-3993df429d52.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] simcha90 commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573153010
##########
File path: superset/utils/core.py
##########
@@ -252,6 +252,12 @@ class ReservedUrlParameters(str, Enum):
STANDALONE = "standalone"
EDIT_MODE = "edit"
+ @classmethod
+ def is_standalone_mode(cls) -> Any:
+ standalone_param = request.args.get(ReservedUrlParameters.STANDALONE.value)
+ standalone = standalone_param != "false" and standalone_param is not None
Review comment:
done
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
# [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=h1) Report
> Merging [#12918](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=desc) (7c2bf4d) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `8.83%`.
> The diff coverage is `65.38%`.
[](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12918 +/- ##
==========================================
+ Coverage 53.06% 61.89% +8.83%
==========================================
Files 489 547 +58
Lines 17314 20173 +2859
Branches 4482 5282 +800
==========================================
+ Hits 9187 12486 +3299
+ Misses 8127 7475 -652
- Partials 0 212 +212
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.89% <65.38%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ontend/src/components/ListViewCard/ImageLoader.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL0ltYWdlTG9hZGVyLnRzeA==) | `86.36% <0.00%> (+11.36%)` | :arrow_up: |
| [...set-frontend/src/components/URLShortLinkButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rQnV0dG9uLmpzeA==) | `100.00% <ø> (ø)` | |
| [...src/dashboard/components/HeaderActionsDropdown.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlckFjdGlvbnNEcm9wZG93bi5qc3g=) | `54.79% <ø> (-12.78%)` | :arrow_down: |
| [...end/src/dashboard/components/StickyVerticalBar.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1N0aWNreVZlcnRpY2FsQmFyLnRzeA==) | `50.00% <ø> (-50.00%)` | :arrow_down: |
| [...rontend/src/explore/components/EmbedCodeButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FbWJlZENvZGVCdXR0b24uanN4) | `80.76% <ø> (ø)` | |
| [...ntend/src/explore/components/ExploreChartPanel.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ2hhcnRQYW5lbC5qc3g=) | `16.17% <ø> (-67.41%)` | :arrow_down: |
| [...nd/src/explore/components/ExploreViewContainer.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlVmlld0NvbnRhaW5lci5qc3g=) | `2.43% <0.00%> (-77.07%)` | :arrow_down: |
| [superset-frontend/src/utils/common.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2NvbW1vbi5qcw==) | `87.27% <ø> (+68.43%)` | :arrow_up: |
| [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `71.65% <ø> (-2.66%)` | :arrow_down: |
| [...rontend/src/views/CRUD/dashboard/DashboardCard.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGFzaGJvYXJkL0Rhc2hib2FyZENhcmQudHN4) | `75.67% <ø> (-0.33%)` | :arrow_down: |
| ... and [477 more](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=footer). Last update [3e0681b...7c2bf4d](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] simcha90 commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r574293729
##########
File path: superset-frontend/src/explore/exploreUtils.js
##########
@@ -99,8 +100,8 @@ export function getExploreLongUrl(
search[key] = extraSearch[key];
});
search.form_data = safeStringify(formData);
- if (endpointType === 'standalone') {
- search.standalone = 'true';
+ if (endpointType === URL_PARAMS.standalone) {
+ search.standalone = '1';
Review comment:
fixed
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
----------------------------------------------------------------
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.
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] simcha90 commented on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-774636177
> How about we introduce two "standalone modes"?
>
> * `standelone=true` or `standlone=1`: hide navs only
> * `standalone=2`: hide navs and title
>
> Let's not give users the choice to hide just the dashboard title. If the request comes back up with a reasonable business need to support it, then we can add it as `standlone=3`.
done
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772889577
thanks for the PR @simcha90 ! User will still be able to add filters after hiding header and edit button since the actions take place in the view mode. is this intentional?
@mihir174 I think this is a valid user need - sharing a dashboard to those who should not be editing it. this solution to me is a bit "hacky" though. any idea?
from Product standpoint, this is a good start, if the code looks good, let's let it in and iterate on the the UX ✅
feature works as expected! ✅
<img width="1787" alt="Screen Shot 2021-02-03 at 3 15 48 PM" src="https://user-images.githubusercontent.com/67837651/106821824-c4bc3a00-6632-11eb-8146-3993df429d52.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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-io edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
# [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=h1) Report
> Merging [#12918](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=desc) (3ad9c36) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `0.35%`.
> The diff coverage is `58.69%`.
[](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12918 +/- ##
==========================================
+ Coverage 53.06% 53.41% +0.35%
==========================================
Files 489 490 +1
Lines 17314 17326 +12
Branches 4482 4490 +8
==========================================
+ Hits 9187 9255 +68
+ Misses 8127 8071 -56
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `53.41% <58.69%> (+0.35%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rontend/src/components/ListView/CardSortSelect.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvQ2FyZFNvcnRTZWxlY3QudHN4) | `78.94% <ø> (ø)` | |
| [...ontend/src/components/ListViewCard/ImageLoader.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL0ltYWdlTG9hZGVyLnRzeA==) | `75.00% <0.00%> (ø)` | |
| [...set-frontend/src/components/URLShortLinkButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rQnV0dG9uLmpzeA==) | `100.00% <ø> (ø)` | |
| [...rset-frontend/src/components/URLShortLinkModal.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rTW9kYWwudHN4) | `77.77% <ø> (ø)` | |
| [...src/dashboard/components/HeaderActionsDropdown.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlckFjdGlvbnNEcm9wZG93bi5qc3g=) | `68.49% <ø> (+0.92%)` | :arrow_up: |
| [...end/src/dashboard/components/StickyVerticalBar.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1N0aWNreVZlcnRpY2FsQmFyLnRzeA==) | `100.00% <ø> (ø)` | |
| [...rontend/src/explore/components/EmbedCodeButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FbWJlZENvZGVCdXR0b24uanN4) | `80.76% <ø> (ø)` | |
| [...ntend/src/explore/components/ExploreChartPanel.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ2hhcnRQYW5lbC5qc3g=) | `83.58% <ø> (ø)` | |
| [superset-frontend/src/utils/common.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2NvbW1vbi5qcw==) | `18.18% <ø> (-0.66%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `79.81% <ø> (+5.50%)` | :arrow_up: |
| ... and [22 more](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=footer). Last update [3e0681b...3ad9c36](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
# [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=h1) Report
> Merging [#12918](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=desc) (3ad9c36) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **decrease** coverage by `0.01%`.
> The diff coverage is `58.69%`.
[](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12918 +/- ##
==========================================
- Coverage 53.06% 53.04% -0.02%
==========================================
Files 489 490 +1
Lines 17314 17326 +12
Branches 4482 4490 +8
==========================================
+ Hits 9187 9191 +4
- Misses 8127 8135 +8
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `53.04% <58.69%> (-0.02%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rontend/src/components/ListView/CardSortSelect.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvQ2FyZFNvcnRTZWxlY3QudHN4) | `78.94% <ø> (ø)` | |
| [...ontend/src/components/ListViewCard/ImageLoader.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL0ltYWdlTG9hZGVyLnRzeA==) | `75.00% <0.00%> (ø)` | |
| [...set-frontend/src/components/URLShortLinkButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rQnV0dG9uLmpzeA==) | `100.00% <ø> (ø)` | |
| [...rset-frontend/src/components/URLShortLinkModal.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVVJMU2hvcnRMaW5rTW9kYWwudHN4) | `77.77% <ø> (ø)` | |
| [...src/dashboard/components/HeaderActionsDropdown.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlckFjdGlvbnNEcm9wZG93bi5qc3g=) | `68.49% <ø> (+0.92%)` | :arrow_up: |
| [...end/src/dashboard/components/StickyVerticalBar.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1N0aWNreVZlcnRpY2FsQmFyLnRzeA==) | `100.00% <ø> (ø)` | |
| [...rontend/src/explore/components/EmbedCodeButton.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FbWJlZENvZGVCdXR0b24uanN4) | `80.76% <ø> (ø)` | |
| [...ntend/src/explore/components/ExploreChartPanel.jsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ2hhcnRQYW5lbC5qc3g=) | `83.58% <ø> (ø)` | |
| [superset-frontend/src/utils/common.js](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2NvbW1vbi5qcw==) | `18.18% <ø> (-0.66%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `79.81% <ø> (+5.50%)` | :arrow_up: |
| ... and [27 more](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=footer). Last update [3e0681b...3ad9c36](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] simcha90 edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-773118838
> Why not call the parameter `standalone`, too, just to be consistent with charts?
`standalone` - hides only top menu bar
`hide_dashboard_hide` - hides only header with edit button
@ktmud I think it dependent on user needs we need give both options to manage dashboard view
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
# [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=h1) Report
> Merging [#12918](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=desc) (a11e8ad) into [master](https://codecov.io/gh/apache/superset/commit/fd2d87340b9741a722b3e7e9a391fe6278d7b3e7?el=desc) (fd2d873) will **decrease** coverage by `0.11%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12918 +/- ##
==========================================
- Coverage 66.98% 66.87% -0.12%
==========================================
Files 1026 489 -537
Lines 50351 28661 -21690
Branches 5189 0 -5189
==========================================
- Hits 33728 19166 -14562
+ Misses 16489 9495 -6994
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.87% <100.00%> (+2.75%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.64% <ø> (ø)` | |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `91.53% <100.00%> (+1.75%)` | :arrow_up: |
| [superset/connectors/druid/models.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC9tb2RlbHMucHk=) | `82.14% <100.00%> (-0.07%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.61% <100.00%> (-0.97%)` | :arrow_down: |
| [superset/models/slice.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NsaWNlLnB5) | `85.87% <100.00%> (+0.40%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `83.39% <100.00%> (+0.19%)` | :arrow_up: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| ... and [551 more](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=footer). Last update [2f6d1ff...a11e8ad](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772889577
thanks for the PR @simcha90 ! User will still be able to add filters after hiding header and edit button since the actions take place in the view mode. is this intended?
@mihir174 I think this is a valid user need - sharing a dashboard to other that should not be editing it. this solution to me is a bit "hacky" though. any idea?
----------------------------------------------------------------
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.
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] simcha90 commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573150341
##########
File path: superset-frontend/src/dashboard/util/getDashboardUrl.ts
##########
@@ -0,0 +1,75 @@
+/**
+ * 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 { URL_PARAMS } from 'src/constants';
+import serializeActiveFilterValues from './serializeActiveFilterValues';
+
+export default function getDashboardUrl(
+ pathname: string,
+ filters = {},
+ hash = '',
+ standalone?: number | null,
+) {
+ const newSearchParams = new URLSearchParams();
+
+ // convert flattened { [id_column]: values } object
+ // to nested filter object
+ newSearchParams.set(
+ URL_PARAMS.preselectFilters,
+ JSON.stringify(serializeActiveFilterValues(filters)),
+ );
+
+ if (standalone) {
+ newSearchParams.set(URL_PARAMS.standalone, standalone.toString());
+ }
+
+ const hashSection = hash ? `#${hash}` : '';
+
+ return `${pathname}?${newSearchParams.toString()}${hashSection}`;
+}
+
+export type UrlParamType = 'string' | 'number' | 'boolean';
+export function getUrlParam(paramName: string, type: 'string'): string;
+export function getUrlParam(paramName: string, type: 'number'): number;
+export function getUrlParam(paramName: string, type: 'boolean'): boolean;
+export function getUrlParam(paramName: string, type: UrlParamType): unknown {
+ const urlParam = new URLSearchParams(window.location.search.substring(1)).get(
+ paramName,
+ );
+ switch (type) {
+ case 'number':
+ if (!urlParam) {
+ return null;
+ }
+ if (urlParam === 'true') {
+ return 1;
+ }
+ if (urlParam === 'false') {
+ return 0;
+ }
+ // eslint-disable-next-line no-case-declarations
+ const parsedNumber = parseInt(urlParam, 10);
Review comment:
done
----------------------------------------------------------------
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.
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] simcha90 commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573587836
##########
File path: superset-frontend/src/utils/common.ts
##########
@@ -44,7 +44,13 @@ export function getParamFromQuery(query, param) {
return null;
}
-export function storeQuery(query) {
+export function storeQuery(query: {
+ dbId: any;
+ title: any;
+ schema: any;
+ autorun: any;
+ sql: any;
+}) {
Review comment:
👍 did as second proposal, created file for `urlUtils`
----------------------------------------------------------------
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.
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] simcha90 commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573154768
##########
File path: superset-frontend/src/dashboard/components/DashboardBuilder.jsx
##########
@@ -225,58 +227,72 @@ class DashboardBuilder extends React.Component {
const childIds = topLevelTabs ? topLevelTabs.children : [DASHBOARD_GRID_ID];
- const barTopOffset = HEADER_HEIGHT + (topLevelTabs ? TABS_HEIGHT : 0);
+ const hideDashboardHeader =
+ getUrlParam(URL_PARAMS.standalone, 'number') === 2;
Review comment:
done
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r572691526
##########
File path: superset-frontend/src/dashboard/util/getDashboardUrl.ts
##########
@@ -0,0 +1,75 @@
+/**
+ * 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 { URL_PARAMS } from 'src/constants';
+import serializeActiveFilterValues from './serializeActiveFilterValues';
+
+export default function getDashboardUrl(
+ pathname: string,
+ filters = {},
+ hash = '',
+ standalone?: number | null,
+) {
+ const newSearchParams = new URLSearchParams();
+
+ // convert flattened { [id_column]: values } object
+ // to nested filter object
+ newSearchParams.set(
+ URL_PARAMS.preselectFilters,
+ JSON.stringify(serializeActiveFilterValues(filters)),
+ );
+
+ if (standalone) {
+ newSearchParams.set(URL_PARAMS.standalone, standalone.toString());
+ }
+
+ const hashSection = hash ? `#${hash}` : '';
+
+ return `${pathname}?${newSearchParams.toString()}${hashSection}`;
+}
+
+export type UrlParamType = 'string' | 'number' | 'boolean';
+export function getUrlParam(paramName: string, type: 'string'): string;
+export function getUrlParam(paramName: string, type: 'number'): number;
+export function getUrlParam(paramName: string, type: 'boolean'): boolean;
+export function getUrlParam(paramName: string, type: UrlParamType): unknown {
+ const urlParam = new URLSearchParams(window.location.search.substring(1)).get(
Review comment:
`URLSearchParams` is [capable of a leading `?`](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/URLSearchParams#parameters) so you don't need `substring(1)` here.
##########
File path: superset-frontend/src/dashboard/util/getDashboardUrl.ts
##########
@@ -0,0 +1,75 @@
+/**
+ * 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 { URL_PARAMS } from 'src/constants';
+import serializeActiveFilterValues from './serializeActiveFilterValues';
+
+export default function getDashboardUrl(
+ pathname: string,
+ filters = {},
+ hash = '',
+ standalone?: number | null,
+) {
+ const newSearchParams = new URLSearchParams();
+
+ // convert flattened { [id_column]: values } object
+ // to nested filter object
+ newSearchParams.set(
+ URL_PARAMS.preselectFilters,
+ JSON.stringify(serializeActiveFilterValues(filters)),
+ );
+
+ if (standalone) {
+ newSearchParams.set(URL_PARAMS.standalone, standalone.toString());
+ }
+
+ const hashSection = hash ? `#${hash}` : '';
+
+ return `${pathname}?${newSearchParams.toString()}${hashSection}`;
+}
+
+export type UrlParamType = 'string' | 'number' | 'boolean';
+export function getUrlParam(paramName: string, type: 'string'): string;
+export function getUrlParam(paramName: string, type: 'number'): number;
+export function getUrlParam(paramName: string, type: 'boolean'): boolean;
+export function getUrlParam(paramName: string, type: UrlParamType): unknown {
Review comment:
This feel like a useful general util. Maybe move to `src/commons/utils/getUrlPram.ts`?
##########
File path: superset/utils/core.py
##########
@@ -252,6 +252,12 @@ class ReservedUrlParameters(str, Enum):
STANDALONE = "standalone"
EDIT_MODE = "edit"
+ @classmethod
+ def is_standalone_mode(cls) -> Any:
Review comment:
```suggestion
@staticmethod
def is_standalone_mode() -> bool:
```
##########
File path: superset/views/core.py
##########
@@ -400,9 +401,11 @@ def slice(self, slice_id: int) -> FlaskResponse: # pylint: disable=no-self-use
endpoint = "/superset/explore/?form_data={}".format(
parse.quote(json.dumps({"slice_id": slice_id}))
)
- param = utils.ReservedUrlParameters.STANDALONE.value
- if request.args.get(param) == "true":
- endpoint += f"&{param}=true"
+
+ is_standalone_mode = ReservedUrlParameters.is_standalone_mode()
+ if is_standalone_mode:
+ param = utils.ReservedUrlParameters.STANDALONE.value
+ endpoint += f"&{param}={is_standalone_mode}"
Review comment:
```suggestion
endpoint += f"&{ReservedUrlParameters.STANDALONE}={is_standalone_mode}"
```
I think this reads better
##########
File path: superset-frontend/src/dashboard/components/DashboardBuilder.jsx
##########
@@ -225,58 +227,72 @@ class DashboardBuilder extends React.Component {
const childIds = topLevelTabs ? topLevelTabs.children : [DASHBOARD_GRID_ID];
- const barTopOffset = HEADER_HEIGHT + (topLevelTabs ? TABS_HEIGHT : 0);
+ const hideDashboardHeader =
+ getUrlParam(URL_PARAMS.standalone, 'number') === 2;
Review comment:
Maybe make `1` and `2` a constant/enum somewhere?
```ts
enum DashboardStandaloneMode {
hideNav = 1,
hideNavAndTitle = 2,
}
```
##########
File path: superset/utils/core.py
##########
@@ -252,6 +252,12 @@ class ReservedUrlParameters(str, Enum):
STANDALONE = "standalone"
EDIT_MODE = "edit"
+ @classmethod
+ def is_standalone_mode(cls) -> Any:
+ standalone_param = request.args.get(ReservedUrlParameters.STANDALONE.value)
+ standalone = standalone_param != "false" and standalone_param is not None
Review comment:
```suggestion
standalone = standalone_param and standalone_param != "false" and standalone_param != "0"
```
##########
File path: superset-frontend/src/dashboard/util/getDashboardUrl.ts
##########
@@ -0,0 +1,75 @@
+/**
+ * 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 { URL_PARAMS } from 'src/constants';
+import serializeActiveFilterValues from './serializeActiveFilterValues';
+
+export default function getDashboardUrl(
+ pathname: string,
+ filters = {},
+ hash = '',
+ standalone?: number | null,
+) {
+ const newSearchParams = new URLSearchParams();
+
+ // convert flattened { [id_column]: values } object
+ // to nested filter object
+ newSearchParams.set(
+ URL_PARAMS.preselectFilters,
+ JSON.stringify(serializeActiveFilterValues(filters)),
+ );
+
+ if (standalone) {
+ newSearchParams.set(URL_PARAMS.standalone, standalone.toString());
+ }
+
+ const hashSection = hash ? `#${hash}` : '';
+
+ return `${pathname}?${newSearchParams.toString()}${hashSection}`;
+}
+
+export type UrlParamType = 'string' | 'number' | 'boolean';
+export function getUrlParam(paramName: string, type: 'string'): string;
+export function getUrlParam(paramName: string, type: 'number'): number;
+export function getUrlParam(paramName: string, type: 'boolean'): boolean;
+export function getUrlParam(paramName: string, type: UrlParamType): unknown {
+ const urlParam = new URLSearchParams(window.location.search.substring(1)).get(
+ paramName,
+ );
+ switch (type) {
+ case 'number':
+ if (!urlParam) {
+ return null;
+ }
+ if (urlParam === 'true') {
+ return 1;
+ }
+ if (urlParam === 'false') {
+ return 0;
+ }
+ // eslint-disable-next-line no-case-declarations
+ const parsedNumber = parseInt(urlParam, 10);
Review comment:
Maybe you can use `Number(urlParam)` to avoid the ESLint error?
##########
File path: superset-frontend/src/dashboard/util/getDashboardUrl.ts
##########
@@ -0,0 +1,75 @@
+/**
+ * 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 { URL_PARAMS } from 'src/constants';
+import serializeActiveFilterValues from './serializeActiveFilterValues';
+
+export default function getDashboardUrl(
+ pathname: string,
+ filters = {},
+ hash = '',
+ standalone?: number | null,
+) {
+ const newSearchParams = new URLSearchParams();
+
+ // convert flattened { [id_column]: values } object
+ // to nested filter object
+ newSearchParams.set(
+ URL_PARAMS.preselectFilters,
+ JSON.stringify(serializeActiveFilterValues(filters)),
+ );
+
+ if (standalone) {
+ newSearchParams.set(URL_PARAMS.standalone, standalone.toString());
+ }
+
+ const hashSection = hash ? `#${hash}` : '';
+
+ return `${pathname}?${newSearchParams.toString()}${hashSection}`;
+}
+
+export type UrlParamType = 'string' | 'number' | 'boolean';
+export function getUrlParam(paramName: string, type: 'string'): string;
+export function getUrlParam(paramName: string, type: 'number'): number;
+export function getUrlParam(paramName: string, type: 'boolean'): boolean;
+export function getUrlParam(paramName: string, type: UrlParamType): unknown {
+ const urlParam = new URLSearchParams(window.location.search.substring(1)).get(
+ paramName,
+ );
+ switch (type) {
+ case 'number':
+ if (!urlParam) {
+ return null;
+ }
+ if (urlParam === 'true') {
+ return 1;
+ }
+ if (urlParam === 'false') {
+ return 0;
+ }
+ // eslint-disable-next-line no-case-declarations
+ const parsedNumber = parseInt(urlParam, 10);
+ if (Number.isInteger(parsedNumber)) {
+ return parsedNumber;
+ }
Review comment:
I think it's OK to allow floats here? The param type is `number`, not `integer`, after all.
But you might want to check and exclude `Number.isNaN` instead.
----------------------------------------------------------------
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.
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] simcha90 edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-773118838
> Why not call the parameter `standalone`, too, just to be consistent with charts?
`standalone` - hides only top menu bar
`hide_dashboard_hide` - hides only header with edit button
@ktmud I think it dependent on user needs we need give both options to manage dashboard view
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-773165957
Why would you want to keep Superset navs while hiding dashboard title? Wouldn't users be able to navigate to other Superset pages in the embedded dashboard? I'm finding it hard to imagine a scenario where this would be desired.
----------------------------------------------------------------
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.
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] simcha90 commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #12918:
URL: https://github.com/apache/superset/pull/12918#discussion_r573151130
##########
File path: superset/utils/core.py
##########
@@ -252,6 +252,12 @@ class ReservedUrlParameters(str, Enum):
STANDALONE = "standalone"
EDIT_MODE = "edit"
+ @classmethod
+ def is_standalone_mode(cls) -> Any:
Review comment:
done
----------------------------------------------------------------
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.
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] mihir174 commented on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
mihir174 commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772932592
Agreed that this is a bit hacky but it's a good first step. We need to think about security/view permission configurations on embed links.
I like the way Google, for example, deals with link sharing:
<img width="635" alt="Screen Shot 2021-02-03 at 4 31 16 PM" src="https://user-images.githubusercontent.com/64227069/106827203-431dd980-663d-11eb-90ad-b197ca6cb22b.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] amitmiran137 commented on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-773178658
@ktmud I agree that the use-case of standalone=false and hide_dashboard_hide=true does not exist
but there are 2 cases that do exist
1. standalone=true & hide_dashboard_hide=false
2. standalone =true & hide_dashboard_hide=true
I can agree that by just setting hide_dashboard_hide=true we can enable standalone but not sure we must enforce it
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.
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-io edited a comment on pull request #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772608461
# [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=h1) Report
> Merging [#12918](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=desc) (a11e8ad) into [master](https://codecov.io/gh/apache/superset/commit/fd2d87340b9741a722b3e7e9a391fe6278d7b3e7?el=desc) (fd2d873) will **decrease** coverage by `0.10%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12918 +/- ##
==========================================
- Coverage 66.98% 66.87% -0.11%
==========================================
Files 1026 489 -537
Lines 50351 28669 -21682
Branches 5189 0 -5189
==========================================
- Hits 33728 19173 -14555
+ Misses 16489 9496 -6993
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.87% <100.00%> (+2.75%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.64% <ø> (ø)` | |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `91.53% <100.00%> (+1.75%)` | :arrow_up: |
| [superset/connectors/druid/models.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC9tb2RlbHMucHk=) | `82.14% <100.00%> (-0.07%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.63% <100.00%> (-0.96%)` | :arrow_down: |
| [superset/models/slice.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NsaWNlLnB5) | `85.87% <100.00%> (+0.40%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `83.39% <100.00%> (+0.19%)` | :arrow_up: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| ... and [545 more](https://codecov.io/gh/apache/superset/pull/12918/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=footer). Last update [2f6d1ff...a11e8ad](https://codecov.io/gh/apache/superset/pull/12918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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 #12918: feat(style): hide dashboard header by url parameter
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12918:
URL: https://github.com/apache/superset/pull/12918#issuecomment-772889577
thanks for the PR @simcha90 ! User will still be able to add filters after hiding header and edit button. is this intended?
@mihir174 I think this is a valid user need - sharing a dashboard to other that should not be editing it. this solution to me is a bit "hacky" though. any idea?
----------------------------------------------------------------
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.
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