Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NcMentionBubble): inject component styles in NcRichContenteditable #5138

Closed
wants to merge 1 commit into from

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Jan 25, 2024

☑️ Resolves

  • Fix NcRichContenteditable: @mention item is not rendered properly in other apps #4901
    • 👎 genSelectTemplate returns a HTML template of NcMentionBubble component with scoped style attribute data-v-123, but these styles are not included in the bundled chunk for some reason
    • 👎 Because of that, NcRichContenteditable renders mention template as expected, but without applied styles
    • 👍 PR extracts NcMentionBubble styles to a mixin, so it could be included in every needed component to its scoped styles like this:
@import '../../assets/NcMentionBubble';
:deep(.mention-bubble) {
	@include mention-bubble;
}

Drawback: because NcMentionBubble is still imported by richeditor mixin, styles in the compiled JS file are duplicated, but it's about 7 style rules, so lesser evil 👿

Tested with Talk chat, Files comments, and should be applicable for other apps

🖼️ Screenshots

🏚️ Before 🏡 After
image image
image image

For Files, name/label is not provided on server-side -> unrelated to this PR

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@Antreesy Antreesy added bug Something isn't working 3. to review Waiting for reviews feature: rich-contenteditable Related to the rich-contenteditable components labels Jan 25, 2024
@Antreesy Antreesy added this to the 8.5.1 milestone Jan 25, 2024
@Antreesy Antreesy self-assigned this Jan 25, 2024
@Antreesy Antreesy force-pushed the fix/4901/mention-bubble-styles branch from f87dd6c to e043e35 Compare January 26, 2024 21:54
@Antreesy Antreesy mentioned this pull request Jan 26, 2024
@Antreesy Antreesy force-pushed the fix/4901/mention-bubble-styles branch from e043e35 to 9bb1519 Compare January 26, 2024 22:10
@susnux
Copy link
Contributor

susnux commented Jan 26, 2024

This is a not that nice work around, the real but seems to be the way we use it + a bug in vite. As vite removes the chunk that imports the css :/
I am currently digging into vite source

@Antreesy
Copy link
Contributor Author

I am currently digging into vite source

https://vitejs.dev/config/build-options.html#build-csscodesplit

Could it be related?

@susnux susnux modified the milestones: 8.5.1, 8.6.0 Jan 26, 2024
@susnux
Copy link
Contributor

susnux commented Jan 26, 2024

No the problem is vite removes chunks that only import css in the generateBundle hook, but we currently inject css only in the renderChunk hook which is called before.
I do not think it is a bug but a decision -> if you remove the chunks in renderChunk you would need to check every chunk twice.

The solution would be to fix the css plugin to also do the import in generateBundle but this might lead to duplicated code, but I will try.

@Antreesy
Copy link
Contributor Author

Superseded by nextcloud-libraries/nextcloud-vite-config#112

@Antreesy Antreesy closed this Jan 29, 2024
@Antreesy Antreesy removed this from the 8.6.0 milestone Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: rich-contenteditable Related to the rich-contenteditable components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NcRichContenteditable: @mention item is not rendered properly in other apps
2 participants