Skip to content

feat: Add office switcher with feature comparison#57555

Merged
juliusknorr merged 2 commits intomasterfrom
feat/office-switcher
Feb 3, 2026
Merged

feat: Add office switcher with feature comparison#57555
juliusknorr merged 2 commits intomasterfrom
feat/office-switcher

Conversation

@juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Jan 14, 2026 •
edited
Loading

Implements an office switcher in the apps management to promote alternatives. For all in one this is hidden as we then have the switcher in nextcloud/all-in-one#7421

nextcloud local_index php_settings_apps_office(Marketing Screenshot)

@juliusknorr juliusknorr requested review from a team as code owners January 14, 2026 21:44
@juliusknorr juliusknorr requested review from CarlSchwan, come-nc, emberfiend, leftybournes, sorbaugh, susnux and szaimen and removed request for a team January 14, 2026 21:44
@juliusknorr juliusknorr added enhancement 3. to review Waiting for reviews labels Jan 14, 2026
@juliusknorr juliusknorr added this to the Nextcloud 33 milestone Jan 14, 2026
@juliusknorr juliusknorr requested a review from elzody January 14, 2026 21:47
@marcoambrosini marcoambrosini mentioned this pull request Jan 15, 2026
Merged
@szaimen szaimen requested review from ShGKme and removed request for szaimen January 15, 2026 11:01
@ShGKme
Copy link
Contributor

ShGKme commented Jan 15, 2026

I'm not sure it is a good idea to add a flag specifically for AIO.
Isn't it better to make it a configuration option, so any environment can disable it if needed?

@juliusknorr
Copy link
Member Author

juliusknorr commented Jan 15, 2026

Isn't it better to make it a configuration option, so any environment can disable it if needed?

No strong opinions about this, if we see other use cases to disable I think yes, otherwise I'd keep it simple that way and it is also what is used in other places like app_api

@come-nc come-nc removed their request for review January 15, 2026 14:31
This was referenced Jan 15, 2026
Merged
Merged
Merged
@juliusknorr juliusknorr moved this to 👀 In review in 📝 Office team Jan 22, 2026
@juliusknorr juliusknorr self-assigned this Jan 22, 2026
@juliusknorr
Copy link
Member Author

juliusknorr commented Jan 22, 2026

Let's keep this check for now, we can add an option if we see other use cases.

@juliusknorr
Copy link
Member Author

juliusknorr commented Jan 22, 2026

I also adjusted the design with feedback from @marcoambrosini to match the one in the all-in-one UI.

elzody
elzody approved these changes Jan 22, 2026
View reviewed changes
Copy link
Contributor

@elzody elzody left a comment

Choose a reason for hiding this comment

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

Looks fine to me. Gave it a quick test and it looks quite nice.

emberfiend
emberfiend approved these changes Jan 22, 2026
View reviewed changes
Copy link

@emberfiend emberfiend left a comment

Choose a reason for hiding this comment

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

One thing that I only discovered by looking at the code was that you could click an already-selected suite to deactivate it. Maybe a text clue, explicit "deactivate" button on an active suite, or 3rd column (or button below?) called "None" or "No suite" could alleviate this. Just a nitpick though, not serious :)

Worth mentioning: I can't test the install -> activate process because the installs hit version compatibility errors, but from what I understand it should be okay.

apps/settings/src/components/AppList/OfficeSuiteSwitcher.vue
transition: all 0.2s ease;
display: flex;
flex-direction: column;

Copy link

@emberfiend emberfiend Jan 22, 2026

Choose a reason for hiding this comment

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

I would add a

	& * {
		cursor: pointer;
	}

here, otherwise the divs and lis lose the cursor when hovered

marcoambrosini
marcoambrosini approved these changes Jan 26, 2026
View reviewed changes
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

I agree with @emberfiend that it would be nice to have a disable button just like in the AIO. The click to disable behavior is a bit counterintuitive.

A few other points:

  • I would use the same Icon we have in AIO for the installed item
Screenshot 2026-01-26 at 12 56 08

  • Since here we have more horizontal space I would probably also append (installed) to the title of the installed suite.

  • I'm curious about how the loading state looks like, is there a spinner in place of the check icon for the selected one?

@juliusknorr
Copy link
Member Author

juliusknorr commented Jan 28, 2026

/compile

@nextcloud-bot nextcloud-bot mentioned this pull request Jan 29, 2026
Merged
@juliusknorr juliusknorr merged commit 8372533 into master Feb 3, 2026
147 of 174 checks passed
@juliusknorr juliusknorr deleted the feat/office-switcher branch February 3, 2026 19:40
@github-project-automation github-project-automation bot moved this from 👀 In review to ☑️ Done in 📝 Office team Feb 3, 2026
@juliusknorr
Copy link
Member Author

juliusknorr commented Feb 3, 2026

/backport to stable33

@backportbot backportbot bot mentioned this pull request Feb 3, 2026
Merged
@susnux
Copy link
Contributor

susnux commented Feb 3, 2026

@juliusknorr why was this merged with red CI? PHPUnit is related.

This errors are caused by this PR:

There were 2 failures:

  1. OCA\Settings\Tests\Controller\AppSettingsControllerTest::testViewApps
    OCP\AppFramework\Services\IInitialState::provideInitialState('usersSettings', [...]): void was not expected to be called more than 4 times.

/home/runner/actions-runner/_work/server/server/apps/settings/lib/Controller/AppSettingsController.php:104
/home/runner/actions-runner/_work/server/server/apps/settings/tests/Controller/AppSettingsControllerTest.php:199

  1. OCA\Settings\Tests\Controller\AppSettingsControllerTest::testViewAppsAppstoreNotEnabled
    OCP\AppFramework\Services\IInitialState::provideInitialState('usersSettings', [...]): void was not expected to be called more than 4 times.

/home/runner/actions-runner/_work/server/server/apps/settings/lib/Controller/AppSettingsController.php:104
/home/runner/actions-runner/_work/server/server/apps/settings/tests/Controller/AppSettingsControllerTest.php:239

susnux
susnux reviewed Feb 3, 2026
View reviewed changes
apps/settings/lib/Controller/AppSettingsController.php
$this->initialState->provideInitialState('appstoreEnabled', $this->config->getSystemValueBool('appstoreenabled', true));
$this->initialState->provideInitialState('appstoreBundles', $this->getBundles());
$this->initialState->provideInitialState('appstoreUpdateCount', count($this->getAppsWithUpdates()));
$this->initialState->provideInitialState('isAllInOne', filter_var(getenv('THIS_IS_AIO'), FILTER_VALIDATE_BOOL));
Copy link
Contributor

@susnux susnux Feb 3, 2026

Choose a reason for hiding this comment

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

Tests need to be adjusted as they count the number of initial state.

@nickvergessen
Copy link
Member

nickvergessen commented Feb 4, 2026

why was this merged with red CI

Change was also performed by #57533 and so it got dropped after the rebase I guess

@nickvergessen nickvergessen mentioned this pull request Feb 4, 2026
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@susnux susnux susnux left review comments

@elzody elzody elzody approved these changes

@marcoambrosini marcoambrosini marcoambrosini approved these changes

@emberfiend emberfiend emberfiend approved these changes

@sorbaugh sorbaugh Awaiting requested review from sorbaugh sorbaugh is a code owner automatically assigned from nextcloud/server-frontend

@leftybournes leftybournes Awaiting requested review from leftybournes leftybournes is a code owner automatically assigned from nextcloud/server-backend

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan CarlSchwan is a code owner automatically assigned from nextcloud/server-backend

@ShGKme ShGKme Awaiting requested review from ShGKme

Assignees

@juliusknorr juliusknorr

Labels

3. to review Waiting for reviews enhancement

Projects

📝 Office team
Status: ☑️ Done

Milestone

Nextcloud 33

Development

Successfully merging this pull request may close these issues.

8 participants

@juliusknorr @ShGKme @susnux @nickvergessen @elzody @marcoambrosini @emberfiend @nextcloud-command