fix(QrcodeStream): styling sometimes missing

[gruhn]

For some users the external style.css is not automatically imported. Now inline all CSS to avoid external CSS files in general.

See #360

[netlify[bot]]

✅ Deploy Preview for vue-qrcode-reader ready!

Name	Link
🔨 Latest commit	b441c79dc91b172ea114e687490efc63aeb11042
🔍 Latest deploy log	https://app.netlify.com/sites/vue-qrcode-reader/deploys/64ece0615486ce000884a0e0
😎 Deploy Preview	https://deploy-preview-362--vue-qrcode-reader.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[gruhn]

@dargmuesli Thanks for your feedback.

I only know other libraries that optionally offer css so that the user can include it or no [1], [2]

As far as I can tell, these libraries provide a lot of aesthetics via CSS (choice of colors, icons, etc). And in that case I also agree that users should opt-in. In contrast, the CSS that comes with QrcodeStream is just a about positioning. It pretty much just defines a z-index stack for the DOM elements in the outermost wrapper div.

For example, if the canvas, where we paint the tracking function, does not perfectly overlay the video element and has the exact same dimensions, then tracking is broken, like here:

Same is true for the other DOM elements. In the Scan Same QR Code More Than Once demo, we use the slot provided by the component to show some micro-interaction after scanning. This also requires that the slot overlays the video element perfectly.

We also have this effect, that when the camera is stopped, we keep showing the last received frame. Also observable in the previously mentioned demo. This works by painting the last frame to a canvas right before the camera stops. Then we swap out the video element by the canvas. To make this look convincing it's also important that the canvas has the same dimension and position as the video element.

So if these CSS rules are missing they really break the component.

I can imagine not many users need the css (even if it's really minimal) as https://github.com/gruhn/vue-qrcode-reader/issues/360 popped up after some time only

I still think most users somehow include the external style.css automatically. We did not document this anywhere how to do this or that this is necessary at all. I don't know how bundlers are doing it, but the demo page for example also includes the CSS seemingly automatically although we import no CSS file anywhere explicitly as far as I can tell:

inline styles are also not really favored when using CSP

Ok I don't quite get what this is about. You mean inline styles are a security risk? Could you elaborate?

[dargmuesli]

My point is simply that people might not want to include a library's CSS at all. I vote for keeping the users' freedom to decide on that themself. An opt-out would be fine as well in that regard of course.

the demo page for example also includes the CSS seemingly automatically

While vite code-splits the css, I think that vitepress bundles vue components itself, including the scoped style.

You mean inline styles are a security risk? Could you elaborate?

Others can do it better than me, see https://stackoverflow.com/a/41925838/4682621

[gruhn]

My point is simply that people might not want to include a library's CSS at all. [...] An opt-out would be fine as well in that regard of course.

But what would be a use case where you don't want to include the CSS? Even if some users have very specific reasons to do that, they can always opt-out by just overriding the styling rules. On the other hand, forgetting to import the styling rules, or having trouble configuring the bundler to do it properly, will be a problem for many users, even if it's properly documented.

While vite code-splits the css, I think that vitepress bundles vue components itself, including the scoped style.

Ah ok, I think you're right. I also just tested the package in a vanilla Vite project and CSS was indeed not imported automatically. Maybe another reason it didn't come up, was that tracking is disabled by default, which makes it less obvious that styling is missing.

Others can do it better than me, see https://stackoverflow.com/a/41925838/4682621

Ok, as far as I understand to exploit this in any way the inline styles still have to be somehow influence-able by some form of user input right? I guess that is strictly not the case here but with active CSP inline styles are blocked all together? That sounds indeed problematic.

I'm not super happy with this solution either. Primarily because I think it makes the code less readable. There is a related issue for at the Vite repo but nobody seems to have a 100% satisfying solution. Either way, I'm strongly in favor of some solution that imports CSS by default.

[dargmuesli]

But what would be a use case where you don't want to include the CSS?

Using the qrcode reader to read a qrcode and remove it once successful, maybe displaying a custom success animation.

they can always opt-out by just overriding the styling rules

I'm just curious: how would that need to be done?

having trouble configuring the bundler to do it properly, will be a problem for many users, even if it's properly documented

I'd argue that adding @import "vue-qrcode-reader/dist/style.css"; is acceptable.

with active CSP inline styles are blocked all together

Exactly.

[dargmuesli]

I'm strongly in favor of some solution that imports CSS by default.

It would be possible to include a component variant with included styles and one without, I think. If you make the current components have styles included, then a QrcodeStreamPlain component could take on the current strategy.

[gruhn]

Using the qrcode reader to read a qrcode and remove it once successful, maybe displaying a custom success animation.

But that is perfectly possible and even easier, with the current styling rules I would say.

I'm just curious: how would that need to be done?

Idk, like this for example:

video { 
   width: 50% !important;
}

It would be possible to include a component variant with included styles and one without, I think.

I don't man :/ The styling rules are tightly coupled to the rest of the implementation. Separating the styling feels arbitrary to me.

You raise a good point with CSP problem. But I would consider the missing CSS export a fairly high priority bug. So if you are not too unhappy with this solution, I would merge this and iterate on it later.

[dargmuesli]

like this for example

But that would keep the library's styles included?

I would consider the missing CSS export a fairly high priority bug

What do you mean with that exactly? That this PR would introduce a fairly high priority bug? Given that the issue the PR intends to resolve does not appear to be of high priority due to it seemingly not affecting many users, should we introduce an issue of higher priority? See note below.

if you are not too unhappy with this solution

With inline-styles I definitely am :laughing: But that must not discourage you per se, we're working in open source after all and anyone could simply pin a past version of the library or maintain a fork without those changes :raised_hands:

---

Possibly important: I did some research and it appears that Vue projects bundle the style automatically, but Vite-only projects don't. Given that this is vue-qrcode-reader I think the project is currently configured as expected. Shouldn't we - instead of doing inline styles - find out what it is that Vue adds to the bundling so that the underlying Vite could be configured properly for those who want to use Vite only?

[dargmuesli]

May I pull in @danielroe here real quick: do you know what makes Vue projects bundle the style.css of Vite libraries properly when at the same time pure Vite projects using vite-plugin-vue only require an @import style.css?

I know we're a bit away from Nuxt with this, but I know you as very reliable when it comes to those questions and you flawlessly helped me to extend my knowledge with all past questions :heart:

[gruhn]

But that would keep the library's styles included?

Yep, it only overrides.

What do you mean with that exactly? That this PR would introduce a fairly high priority bug?

In my opinion the styling is a fundamental part of the component. Previously, no action was required to include it. Now it is. That's a breaking change and since that was unintended I would call it a pretty sewer bug.

it appears that Vue projects bundle the style automatically, but Vite-only projects don't. [...] Shouldn't we - instead of doing inline styles - find out what it is that Vue adds.

Ok that's interesting. If that also amounts to importing the CSS by default, then I would also prefer that. But if not, inline styles shouldn't break "normal" Vue projects either, right? So, the inline-style approach would at least be compatible.

[danielroe]

My own recommendation (apologies) would be to ship the SFC (if it's not suitable to remove the existing JS files, you could add the SFC as well), so the end user's project could bundle and compile the SFC appropriately. In a Nuxt or Vite Vue project, for example, that would not necessarily result in inlining the style but extracting it into the existing CSS file.

I was unaware that Vite projects might automatically load style.css files but would be happy to be corrected.

[gruhn]

@danielroe Thanks for jumping in. So you suggest to not inline the CSS, right? Not sure if you read the entire thread. Here is my argument in summary:

The CSS is very minimal but crucial. Without it the component does not behave correctly (it's not about aesthetics). I'd say in 99% of cases, users need these styling rules, so the strong default would be to apply them automatically.

Is there any other way to guarantee this, other then CSS Inlining?

[dargmuesli]

Previously, no action was required to include it. Now it is.

Still no action seems to be required for Vue projects for which this library is made, right? Only plain Vite projects seem to need some more configuration, which simply was not in scope when this library targeted Vue 2, I think. So I would not call it a pretty severe bug.

inline styles shouldn't break "normal" Vue projects

They would still compile, but at the same time they'd expose those projects to potentially/practically unwanted behavior that should at least be opt-out-able, I think.

[danielroe]

I read the entire thread, and based on what you say, I understand why you need styling.

By shipping an SFC and advocating importing this instead, the styling will be included in the build output in the appropriate place (an external CSS file). It also ensures that the component is compiled with the same version of vue that the rest of their project uses.

It feels like this solution would resolve both @dargmuesli's concerns with CSP, and your concerns that the styling is applied.

But, if you prefer not to ship the SFC, I think using style attributes is reasonable, as you've done here. Possibly you could even add a prop to allow disabling these attributes as an escape hatch. The other way to ensure styling is applied would be to add a step to the README telling users to import it.

[dargmuesli]

shipping an SFC

I like that approach. To be fair, I thought those would be part of the output already. Thank you @danielroe for the detailed comment! :rocket:

[gruhn]

shipping an SFC

Sounds good, if that's compatible with all bundlers. @dargmuesli you know how to configure this?

I would still like to support users who just import via <script> tag. Maybe we can somehow inline the styles only for UMD modules or something...

[dargmuesli]

Sure, you just need to add src to the files array inside the package.json. I went ahead and updated #361 to include that.

[gruhn]

I'd like to thoroughly test this with different bundlers and direct script include. But I'm on vacation for the next two week. So as a temporary solutions, I'd like to merge this inline style solution for now. I promise we'll get back to it.

[github-actions[bot]]

:tada: This PR is included in version 5.3.3 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[dargmuesli]

Hm, I have to bend my mind a bit to understand. Why not ship #361?

[dargmuesli]

When will we get back to the issue of inline styles as promised? :see_no_evil:

[gruhn]

Sorry, I’ve just been lazy to deal with it until now :S