This is an automated email from the ASF dual-hosted git repository.
suddjian pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new cf15fe0 fix(dashboard): custom css should be removed on unmount
(#15025)
cf15fe0 is described below
commit cf15fe0d03529432e1ef1741036ab5b60af197d2
Author: David Aaron Suddjian <[email protected]>
AuthorDate: Mon Jun 7 14:04:56 2021 -0700
fix(dashboard): custom css should be removed on unmount (#15025)
* fix(dashboard): custom css should be removed on unmount
* better comment
* remove unnecessary typecast
* move type to top level scope
---
.../src/dashboard/containers/DashboardPage.tsx | 40 +++++++++-------------
.../{injectCustomCss.js => injectCustomCss.ts} | 36 +++++++++++++------
2 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx
b/superset-frontend/src/dashboard/containers/DashboardPage.tsx
index 580df62..e5897d1 100644
--- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx
+++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx
@@ -26,7 +26,6 @@ import {
useDashboardDatasets,
} from 'src/common/hooks/apiResources';
import { ResourceStatus } from 'src/common/hooks/apiResources/apiResources';
-import { usePrevious } from 'src/common/hooks/usePrevious';
import { hydrateDashboard } from 'src/dashboard/actions/hydrate';
import injectCustomCss from 'src/dashboard/util/injectCustomCss';
@@ -42,14 +41,10 @@ const DashboardContainer = React.lazy(
const DashboardPage: FC = () => {
const dispatch = useDispatch();
const { idOrSlug } = useParams<{ idOrSlug: string }>();
- const [isLoaded, setLoaded] = useState(false);
+ const [isHydrated, setHydrated] = useState(false);
const dashboardResource = useDashboard(idOrSlug);
const chartsResource = useDashboardCharts(idOrSlug);
const datasetsResource = useDashboardDatasets(idOrSlug);
- const isLoading = [dashboardResource, chartsResource, datasetsResource].some(
- resource => resource.status === ResourceStatus.LOADING,
- );
- const wasLoading = usePrevious(isLoading);
const error = [dashboardResource, chartsResource, datasetsResource].find(
resource => resource.status === ResourceStatus.ERROR,
)?.error;
@@ -57,16 +52,22 @@ const DashboardPage: FC = () => {
useEffect(() => {
if (dashboardResource.result) {
document.title = dashboardResource.result.dashboard_title;
+ if (dashboardResource.result.css) {
+ // returning will clean up custom css
+ // when dashboard unmounts or changes
+ return injectCustomCss(dashboardResource.result.css);
+ }
}
+ return () => {};
}, [dashboardResource.result]);
+ const shouldBeHydrated =
+ dashboardResource.status === ResourceStatus.COMPLETE &&
+ chartsResource.status === ResourceStatus.COMPLETE &&
+ datasetsResource.status === ResourceStatus.COMPLETE;
+
useEffect(() => {
- if (
- wasLoading &&
- dashboardResource.status === ResourceStatus.COMPLETE &&
- chartsResource.status === ResourceStatus.COMPLETE &&
- datasetsResource.status === ResourceStatus.COMPLETE
- ) {
+ if (shouldBeHydrated) {
dispatch(
hydrateDashboard(
dashboardResource.result,
@@ -74,20 +75,13 @@ const DashboardPage: FC = () => {
datasetsResource.result,
),
);
- injectCustomCss(dashboardResource.result.css);
- setLoaded(true);
+ setHydrated(true);
}
- }, [
- dispatch,
- wasLoading,
- dashboardResource,
- chartsResource,
- datasetsResource,
- ]);
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [shouldBeHydrated]);
if (error) throw error; // caught in error boundary
-
- if (!isLoaded) return <Loading />;
+ if (!isHydrated) return <Loading />;
return <DashboardContainer />;
};
diff --git a/superset-frontend/src/dashboard/util/injectCustomCss.js
b/superset-frontend/src/dashboard/util/injectCustomCss.ts
similarity index 66%
rename from superset-frontend/src/dashboard/util/injectCustomCss.js
rename to superset-frontend/src/dashboard/util/injectCustomCss.ts
index 4f6238a..b7db2b0 100644
--- a/superset-frontend/src/dashboard/util/injectCustomCss.js
+++ b/superset-frontend/src/dashboard/util/injectCustomCss.ts
@@ -16,19 +16,31 @@
* specific language governing permissions and limitations
* under the License.
*/
-export default function injectCustomCss(css) {
+
+function createStyleElement(className: string) {
+ const style = document.createElement('style');
+ style.className = className;
+ style.type = 'text/css';
+ return style;
+}
+
+// The original, non-typescript code referenced `style.styleSheet`.
+// I can't find what sort of element would have a styleSheet property,
+// so have created this type to satisfy TS without changing behavior.
+type MysteryStyleElement = {
+ styleSheet: {
+ cssText: string;
+ };
+};
+
+export default function injectCustomCss(css: string) {
const className = 'CssEditor-css';
const head = document.head || document.getElementsByTagName('head')[0];
- let style = document.querySelector(`.${className}`);
+ const style: HTMLStyleElement =
+ document.querySelector(`.${className}`) || createStyleElement(className);
- if (!style) {
- style = document.createElement('style');
- style.className = className;
- style.type = 'text/css';
- }
-
- if (style.styleSheet) {
- style.styleSheet.cssText = css;
+ if ('styleSheet' in style) {
+ (style as MysteryStyleElement).styleSheet.cssText = css;
} else {
style.innerHTML = css;
}
@@ -45,4 +57,8 @@ export default function injectCustomCss(css) {
*/
head.appendChild(style);
+
+ return function removeCustomCSS() {
+ style.remove();
+ };
}
