paranext / paranext-core

Electron client, extension host, and C# library for Paranext
https://paranext.github.io/paranext-core/
MIT License
17 stars 2 forks source link

Enable Paranext core component styles in web views #179

Closed tjcouch-sil closed 1 year ago

tjcouch-sil commented 1 year ago

Right now, the only way we are getting styles in web views is by copying the material ui styles and putting them in a web view stylesheet. Either pass down styles from the renderer into each iframe or package paranext core component styles in with the components (which would hopefully be accomplished by #139

irahopkinson commented 1 year ago

Since #139 is not yet done nor even in the current sprint, investigate passing styles to each iframe.

Assumptions: this is a react only problem.

Questions:

tjcouch-sil commented 1 year ago

Agreed, I believe this is a React-only problem as well.

139 is older than this issue and lower priority. If the best way to solve this issue is to create an npm library, do not avoid it just because this issue isn't #139. If you want to go with a different strategy, though, whatever works best.

I think passing styles down might be adding unnecessary complications and fragility to the styles, so I recommend investigating making a library first. Basically I think passing down styles would mean creating a hook that listens for changes in styles in the parent frame and adjusts them in the child. I made useStyle to use a style string in the POC, but it does not do what we need it to do.

I think potentially the easier and more efficient solution would be to bundle the components up into a library. That way, styles are applied in a very expected and normal fashion (required styles are carried in through the library's JS) unlike passing all styles from the parent to the child and updating anytime styles in the parent change.

There are a couple of complications I see potentially happening... If we depend on the papi in these components, that creates a circular dependency because the components are on papi but using papi. Not sure what to do about that to be honest. Maybe putting the hooks and context into this library would help somewhat...? But it would still have the circular dependency. Interesting 🤔

I think including MUI styles in the webview CSS would mean we would have to update the styles manually with every update to MUI or every additional MUI component we use. I think a solution similar to we have done as mentioned in the issue regarding copying styles is a very error-prone, temporary solution.

It would be best to make the solution MUI-independent as we already include and may subsequently include even more libraries other than MUI.

irahopkinson commented 1 year ago

Future TODO:

irahopkinson commented 1 year ago

Also completes #139.

irahopkinson commented 1 year ago

WebView bundle size is controlled by extensions\vite\vite-web-view.config.ts. The papi-component is ~2.2MB because it includes source maps but even by not generating source maps there, they are still generated by extensions\vite\vite-web-view.config.ts. It is possible to remove them for an individual package something like this:

// vite-web-view.config.ts
export default {
  // Other Vite configuration options...

  rollupOptions: {
    output: {
      sourcemap: true,
      sourcemapExcludeSources: true,
    },
    // Exclude source maps for a specific package
    plugin: [
      {
        name: 'exclude-source-maps-plugin',
        generateBundle(_, bundle) {
          for (const fileName in bundle) {
            if (fileName.includes('papi-components')) {
              bundle[fileName].map = undefined;
            }
          }
        },
      },
    ],
  },
};

The rollupOptions.output.sourcemap option is set to true to enable the generation of source maps for all the packages except the specific one. The rollupOptions.output.sourcemapExcludeSources option is set to true to exclude the actual source code from the generated source maps.

The rollupOptions.plugin section includes a custom plugin that loops through the generated bundle and sets the map property to undefined (original suggestion was null) for the specific package, effectively excluding its source maps.

By configuring Vite in this way, you should be able to prevent source maps from being added for the particular package while still generating them for everything else.