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

[next] chore(master): merge master into next #5184

Merged
merged 15 commits into from
Jan 31, 2024
Merged

Conversation

raimund-schluessler
Copy link
Contributor

☑️ Resolves

  • Merges master into next.

@ShGKme I didn't get #5178 to work properly with vue 3 yet. This needs some more work.

mejo- and others added 11 commits January 30, 2024 10:44
E.g. the file reference widget should work with links to Nextcloud on
localhost.

Also add some basic unit tests to prevent regressions with the regexes
in the future.

Signed-off-by: Jonas <jonas@freesources.org>
…attern

fix(NcRichText): Make URL_PATTERN match localhost and URLs with ports
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Co-authored-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Co-authored-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
…tom-children

fix(NcActions): hotfix for custom children
… on system default theme

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…ssibility_to_change_a_color_theme_for_native_datetime_picker-on-default-theme

Create possibility to change a color theme for native datetime picker on system default theme
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@raimund-schluessler raimund-schluessler added this to the 9.0.0-alpha.1 milestone Jan 30, 2024
@raimund-schluessler raimund-schluessler added 2. developing Work in progress vue 3 Related to the vue 3 migration labels Jan 30, 2024
@ShGKme
Copy link
Contributor

ShGKme commented Jan 30, 2024

@ShGKme I didn't get #5178 to work properly with vue 3 yet. This needs some more work.

The issue is not related to this PR. Such usage of NcActions didn't work before as well, because findActions cannot find actions from sub-components (they need to be rendered first). So we have the same issue that we always had in Vue 2 (but more noticeable).

So this PR only adds an example of this limitation to the docs.

We need to find a more generic solution to solve #5171

@@ -1091,7 +1218,7 @@ export default {
* @return {boolean}
*/
isValidSingleAction(action) {
return ['NcActionButton', 'NcActionLink', 'NcActionRouter'].includes(this.getActionName(action))
return ['NcActionButton', 'NcActionLink', 'NcActionRouter'].some(valid => this.getActionName(action).startsWith(valid))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change to startsWith?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought we could introduce a naming convention for these wrappers. So if the components name starts with NcActions, we just pass it on. But that wasn't fully thought through and didn't work yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

As an alternative, we could ask to add a custom option and check $options.

But this won't help in a general case. Developers may make wrappers for a list of actions

<template>
  <div>
    <NcActionButton />
    <NcActionInput />
  </div>
</template>

Or dynamic actions. Like, I dunno,

<template>
  <NcActionButton v-if="iLikeButtons" /> 
  <NcActionInput v-else-if="iDont" />
</template>

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@ShGKme ShGKme Jan 30, 2024

Choose a reason for hiding this comment

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

Have this in forms:

This will work. I meant when those <template>...</template> are templates of new custom components.

<template>
  <NcActionButton v-if="iLikeButtons" /> 
  <NcActionInput v-else-if="iDont" />
</template>

<script>
export default {
  name: 'MyAction'
}
</script>  

and then

<template>
  <NcActions>
    <MyAction />
  </NcActions>
</template>

@raimund-schluessler
Copy link
Contributor Author

Yes, this didn't work at all with next. But I thought we should at least get the same behavior as for master. But due to findActions, wrapped actions do not show up at all.

@ShGKme
Copy link
Contributor

ShGKme commented Jan 30, 2024

Yes, this didn't work at all with next. But I thought we should at least get the same behavior as for master. But due to findActions, wrapped actions do not show up at all.

I have found a small number of apps that require this feature. Text, Mail. I hope, we will find a solution until they migrate =D

transifex-integration bot and others added 2 commits January 30, 2024 19:57
100% translated source file: 'l10n/messages.pot'
on 'ar'.
@raimund-schluessler
Copy link
Contributor Author

Ok, I would propose that we do a simple merge of master into next here without trying to fix the underlying problem and then proceed to find a solution in a new PR. After all, it doesn't work in next anyway, so we don't make it worse.

@raimund-schluessler raimund-schluessler force-pushed the chore/noid/merge-master-next branch 2 times, most recently from 2196317 to 514bbe1 Compare January 30, 2024 20:18
@raimund-schluessler raimund-schluessler marked this pull request as ready for review January 30, 2024 20:23
@raimund-schluessler
Copy link
Contributor Author

Since next uses vitest to run the tests, we had to install @vitest/coverage-v8. I did this and also regenerated package-lock.json to get the latest dependencies.

@raimund-schluessler raimund-schluessler added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 30, 2024
@susnux susnux merged commit ee9f6f3 into next Jan 31, 2024
18 checks passed
@susnux susnux deleted the chore/noid/merge-master-next branch January 31, 2024 10:35
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 vue 3 Related to the vue 3 migration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants