Skip to content

Conversation

@ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Nov 20, 2025

Summary

  • Prerequirement: build: correct render build url in Vite for chunks/assets in dist #56554
  • Migrates the app to Vue 3
  • Vue 2 compatible changes are done in an individual commit, backportable
  • Dirty workarounds:
    • AccountMenuEntry is not fully compatible with Vue 3, expecting menu items to be replaced when mounted in place, not to be mounted inside the entry
      • Requires changes in core API to support it
    • User statuses are copied from user_status to core to avoid illegal import (core should not depend on an app)
    • Some box-sizing: border-box added

Checklist

@ShGKme ShGKme self-assigned this Nov 20, 2025
@ShGKme ShGKme force-pushed the chore/user_status--vue3 branch 2 times, most recently from fa9056c to b837912 Compare November 20, 2025 11:53
@ShGKme ShGKme added this to the Nextcloud 33 milestone Nov 20, 2025
@ShGKme ShGKme added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 20, 2025
@ShGKme ShGKme changed the base branch from master to fix/build-vite-chunks November 20, 2025 12:30
@ShGKme ShGKme force-pushed the chore/user_status--vue3 branch from b837912 to 5ca53f4 Compare November 20, 2025 12:31
@ShGKme ShGKme requested a review from Antreesy November 20, 2025 12:34
@ShGKme ShGKme force-pushed the fix/build-vite-chunks branch from 06cbc16 to d54009f Compare November 20, 2025 14:09
Base automatically changed from fix/build-vite-chunks to master November 20, 2025 23:17
@ShGKme ShGKme force-pushed the chore/user_status--vue3 branch 2 times, most recently from b78f534 to 2515803 Compare November 21, 2025 16:44
@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 21, 2025

/compile

@ShGKme ShGKme marked this pull request as ready for review November 21, 2025 17:44
@ShGKme ShGKme requested review from a team as code owners November 21, 2025 17:44
@ShGKme ShGKme requested review from Altahrim, CarlSchwan, nfebe, sorbaugh and szaimen and removed request for a team November 21, 2025 17:44
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

🐘

@susnux
Copy link
Contributor

susnux commented Nov 25, 2025

/compile rebase

@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 25, 2025

cc @Antreesy as an app owner

@susnux susnux force-pushed the chore/user_status--vue3 branch from b03e6f9 to f008810 Compare November 25, 2025 11:47
@susnux
Copy link
Contributor

susnux commented Nov 25, 2025

Added a commit because lint is red unrelated - this will be fixed in a different PR.
So just baseline it here like we did in the other case.
See: ae13758

@susnux
Copy link
Contributor

susnux commented Nov 25, 2025

Proper fix: #56660

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.

Works with mentioned changes

@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 25, 2025

I was sure, I did all these changes...
I remember fixing value->modelValue before migration and installing debounce v2 for compatibility ._.

@ShGKme ShGKme force-pushed the chore/user_status--vue3 branch from ae13758 to 8493eeb Compare November 25, 2025 18:29
@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 25, 2025

  • Rebased onto master
  • Cleared chore(deps): add [email protected] to be debounce only and in the correct version (2 instead of 3, no Vue version change)
  • Added fixup with events casing + event handling change
  • Added fixup with CSS fix

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.

Looks good now!

'settings-admin': resolve(import.meta.dirname, 'apps/user_ldap/src', 'settings-admin.ts'),
},
user_status: {
menu: resolve(import.meta.dirname, 'apps/user_status/src', 'menu.js'),
Copy link
Member

Choose a reason for hiding this comment

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

No typescript ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is another big chunk of work, I will tackle it later and separately

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 goal of this PR is to migrate the app to Vue 3 with minimal changes. There is much to refactor in this app, but it could be done before and can be done after the Vue 3 migration

@ShGKme ShGKme force-pushed the chore/user_status--vue3 branch from 8493eeb to ddece38 Compare November 26, 2025 09:44
@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 26, 2025

  • Rebased with fixup squash
  • Removed @susnux commit — it was conflicting and seemed unneeded in this PR after changes in another PR

@ShGKme ShGKme enabled auto-merge November 26, 2025 09:45
@susnux
Copy link
Contributor

susnux commented Nov 26, 2025

Removed @susnux commit — it was conflicting and seemed unneeded in this PR after changes in another PR

Yes, thank you :)

ShGKme and others added 4 commits November 26, 2025 14:05
Signed-off-by: Grigorii K. Shartsev <[email protected]>
Signed-off-by: Maksim Sukharev <[email protected]>
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@Antreesy Antreesy force-pushed the chore/user_status--vue3 branch from ddece38 to 8ca4a7a Compare November 26, 2025 13:11
@Antreesy
Copy link
Contributor

/compile

Signed-off-by: nextcloud-command <[email protected]>
@susnux
Copy link
Contributor

susnux commented Nov 26, 2025

grafik

I am always impressed about that, but I guess its only source maps 😅

@nickvergessen nickvergessen merged commit 38792c8 into master Nov 26, 2025
217 of 234 checks passed
@nickvergessen nickvergessen deleted the chore/user_status--vue3 branch November 26, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants