From f113dd5a1ddea58a2f3e94a15c7075a95fee823e Mon Sep 17 00:00:00 2001 From: Kai Vandivier <49666798+KaiVandivier@users.noreply.github.com> Date: Tue, 10 Dec 2024 15:00:17 +0100 Subject: [PATCH] fix(plugin): clean up resize observer and handle sonarqube warnings (#898) * refactor: use css modules in example app * chore: remove finished todos in makeViteConfig * chore: add jira issues to todos * refactor: simplify hosted location string ternary * refactor: remove always truthy condition * fix(plugin): remove resize observer on cleanup * chore: import order * fix: consistent ref * chore: style --- cli/config/makeViteConfig.mjs | 4 ++-- cli/src/commands/deploy.js | 1 + cli/src/commands/start.js | 7 ++++--- cli/src/lib/compiler/compile.js | 1 + examples/simple-app/src/Alerter.jsx | 11 ++--------- examples/simple-app/src/Alerter.module.css | 6 ++++++ examples/simple-app/src/App.jsx | 5 ++--- examples/simple-app/src/App.module.css | 10 ++++++++++ examples/simple-app/src/App.style.js | 14 -------------- shell/src/PluginLoader.jsx | 11 +++++++---- 10 files changed, 35 insertions(+), 35 deletions(-) create mode 100644 examples/simple-app/src/Alerter.module.css create mode 100644 examples/simple-app/src/App.module.css delete mode 100644 examples/simple-app/src/App.style.js diff --git a/cli/config/makeViteConfig.mjs b/cli/config/makeViteConfig.mjs index 97a48e190..c393a53c8 100644 --- a/cli/config/makeViteConfig.mjs +++ b/cli/config/makeViteConfig.mjs @@ -25,7 +25,7 @@ import dynamicImport from 'vite-plugin-dynamic-import' * merged with the main config if the `--allowJsxInJs` flag is passed * to the CLI * - * todo: deprecate -- this config has a performance cost, especially on startup + * ! deprecated -- this config has a performance cost, especially on startup */ const jsxInJsConfig = { plugins: [ @@ -178,7 +178,7 @@ export default ({ paths, config, env, host, force, allowJsxInJs }) => { dynamicImport(), react({ babel: { plugins: ['styled-jsx/babel'] }, - // todo: deprecate with other jsx-in-js config + // ! deprecated with other jsx-in-js config // This option allows HMR of JSX-in-JS files, // but it isn't possible to add with merge config: jsxRuntime: allowJsxInJs ? 'classic' : 'automatic', diff --git a/cli/src/commands/deploy.js b/cli/src/commands/deploy.js index 2798c5ae9..c600b2399 100644 --- a/cli/src/commands/deploy.js +++ b/cli/src/commands/deploy.js @@ -152,6 +152,7 @@ const handler = async ({ cwd = process.cwd(), timeout, ...params }) => { } // todo: modify for multiple/named plugins + // https://dhis2.atlassian.net/browse/LIBS-394 const appUrl = constructAppUrl({ baseUrl: dhis2Config.baseUrl, config, diff --git a/cli/src/commands/start.js b/cli/src/commands/start.js index b101d36b8..f6a6469b9 100644 --- a/cli/src/commands/start.js +++ b/cli/src/commands/start.js @@ -161,11 +161,12 @@ const handler = async ({ }) const server = await createServer(viteConfig) - const location = config.entryPoints.plugin - ? config.entryPoints.app + let location = '' + if (config.entryPoints.plugin) { + location = config.entryPoints.app ? ' at / and /plugin.html' : ' at /plugin.html' - : '' + } reporter.print( `The app ${chalk.bold( diff --git a/cli/src/lib/compiler/compile.js b/cli/src/lib/compiler/compile.js index 3cc84898d..e68744b52 100644 --- a/cli/src/lib/compiler/compile.js +++ b/cli/src/lib/compiler/compile.js @@ -142,6 +142,7 @@ const compile = async ({ inputDir: paths.src, outputDir: outDir, // todo: handle lib compilations with Vite + // https://dhis2.atlassian.net/browse/LIBS-722 processFileCallback: isAppType ? copyFile : compileFile, watch, }), diff --git a/examples/simple-app/src/Alerter.jsx b/examples/simple-app/src/Alerter.jsx index f7973eee0..f7077300b 100644 --- a/examples/simple-app/src/Alerter.jsx +++ b/examples/simple-app/src/Alerter.jsx @@ -1,6 +1,7 @@ import { useAlert } from '@dhis2/app-runtime' import { InputField, CheckboxField, Button } from '@dhis2/ui' import React, { useState } from 'react' +import styles from './Alerter.module.css' export const Alerter = () => { const [message, setMessage] = useState('') @@ -12,7 +13,7 @@ export const Alerter = () => { ) return ( -
+
{ /> -
) } diff --git a/examples/simple-app/src/Alerter.module.css b/examples/simple-app/src/Alerter.module.css new file mode 100644 index 000000000..9ff151b25 --- /dev/null +++ b/examples/simple-app/src/Alerter.module.css @@ -0,0 +1,6 @@ +.flexContainer { + display: flex; + align-items: center; + justify-content: space-between; + width: 500px; +} diff --git a/examples/simple-app/src/App.jsx b/examples/simple-app/src/App.jsx index ee37f898c..78bf15760 100644 --- a/examples/simple-app/src/App.jsx +++ b/examples/simple-app/src/App.jsx @@ -2,7 +2,7 @@ import { useDataQuery } from '@dhis2/app-runtime' import moment from 'moment' import React from 'react' import { Alerter } from './Alerter.jsx' -import style from './App.style.js' +import styles from './App.module.css' import i18n from './locales/index.js' const query = { @@ -14,8 +14,7 @@ const query = { const Component = () => { const { error, loading, data } = useDataQuery(query) return ( -
- +
{error && ERROR} {loading && ...} {data && ( diff --git a/examples/simple-app/src/App.module.css b/examples/simple-app/src/App.module.css new file mode 100644 index 000000000..3ee1159e0 --- /dev/null +++ b/examples/simple-app/src/App.module.css @@ -0,0 +1,10 @@ +.appContainer { + width: 100%; + height: 100%; + + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + font-size: 1rem; +} \ No newline at end of file diff --git a/examples/simple-app/src/App.style.js b/examples/simple-app/src/App.style.js deleted file mode 100644 index d728f2bbe..000000000 --- a/examples/simple-app/src/App.style.js +++ /dev/null @@ -1,14 +0,0 @@ -import css from 'styled-jsx/css' - -export default css` - div { - width: 100%; - height: 100%; - - display: flex; - flex-direction: column; - align-items: center; - justify-content: center; - font-size: 1rem; - } -` diff --git a/shell/src/PluginLoader.jsx b/shell/src/PluginLoader.jsx index c583cb463..ee1178df6 100644 --- a/shell/src/PluginLoader.jsx +++ b/shell/src/PluginLoader.jsx @@ -15,13 +15,16 @@ const PluginResizeInner = ({ const innerDivRef = useRef() useEffect(() => { if (divRef && divRef.current && resizePluginHeight) { + const container = divRef.current const resizeObserver = new ResizeObserver(() => { // the additional pixels currently account for possible horizontal scroll bar - if (resizePluginHeight) { - resizePluginHeight(divRef.current.offsetHeight + 20) - } + resizePluginHeight(container.offsetHeight + 20) }) - resizeObserver.observe(divRef.current) + resizeObserver.observe(container) + return () => { + resizeObserver.unobserve(container) + resizeObserver.disconnect() + } } }, [resizePluginHeight])