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: Leftovers of name prop migration + fix @ in user ids (richEditor) #4222

Merged
merged 1 commit into from Jun 27, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jun 15, 2023

☑️ Resolves

The first commit solves some issues / leftovers where the property was renamed but the example code was not changed.
The second commit solves an issues were mentioning users with @ in there userid was not possible (@ was removed).
Also added a note about the property renaming on the changelog previously only name -> title was mentioned and not label -> title.

New tests included.

🖼️ Screenshots

🏚️ Before 🏡 After
image image

🏁 Checklist

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

@susnux susnux added bug Something isn't working 3. to review Waiting for reviews feature: rich-contenteditable Related to the rich-contenteditable components labels Jun 15, 2023
@susnux susnux added this to the 8.0.0 milestone Jun 15, 2023
@susnux susnux force-pushed the fix/menu-bubble-at-in-user-name branch from 4e92aae to 23c8ab6 Compare June 15, 2023 19:40
@@ -51,7 +51,7 @@ export default {
name: 'NcAutoCompleteResult',

props: {
label: {
title: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was intentional, because we agreed to use the term title only if if relates to the HTML attribute title, which it does not here.
Is there a deeper problem with renaming the prop here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the problem was that the property of the menububble was renamed from label to title, but the menububble and the autocompleteresult share the same data.

Also I think that in this case title is the more logical name for this property usage.

@nickvergessen nickvergessen requested review from Antreesy and removed request for nickvergessen June 22, 2023 08:02
…bel` to `title`

* Also fix the mixin to allow `@` within userids

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the fix/menu-bubble-at-in-user-name branch from 23c8ab6 to cb532ca Compare June 26, 2023 12:26
@susnux susnux requested a review from Antreesy June 26, 2023 12:26
@susnux
Copy link
Contributor Author

susnux commented Jun 26, 2023

@Antreesy nice! Add the title prop :)

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦭

@susnux susnux merged commit 1dcaadf into master Jun 27, 2023
16 checks passed
@susnux susnux deleted the fix/menu-bubble-at-in-user-name branch June 27, 2023 18:56
@skjnldsv skjnldsv mentioned this pull request Jun 28, 2023
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.

User mention in comment field does not work for users with @ in name
4 participants