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/09 09:07:02 UTC
[GitHub] [superset] ktmud commented on a change in pull request #12918: feat(style): hide dashboard header by url parameter
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