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

Automatically show oC10 apps in app switcher menu #5996

Merged
merged 8 commits into from
Nov 11, 2021
Merged

Conversation

JammingBen
Copy link
Collaborator

@JammingBen JammingBen commented Nov 8, 2021

With this change, oC10 apps will be listed in the app switcher menu.

2 things that are not so good with the current approach:

  • We only have full URLs to the icons. The config on the other hand expects a string to load the icons. Therefore the application icon is the default for now. -> we use the extension icon
  • The app name we get from \OC::$server->getNavigationManager()->getAll() is already translated. To overcome this, we currently load the app + 3 different l10n instances to provide proper solutions based on the original name... super ugly. Is there a way to use the already translated string for OC web? -> That's okay for now.

Related Issue

Screenshots:

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@JammingBen JammingBen changed the title Show oC10 apps in app switcher menu Automatically show oC10 apps in app switcher menu Nov 8, 2021
@tbsbdr
Copy link
Contributor

tbsbdr commented Nov 8, 2021

oh wow, thx @JammingBen!

about the icon:
our current "application" icon would be missleading as is symbolises settings.
I would suggest to have one of these icons in the app switcher for the time beeing:

which one would you prefer @JammingBen @kulmann (I favour "Extension")

@AlexAndBear
Copy link
Contributor

I vote for Extension

@JammingBen
Copy link
Collaborator Author

I also prefer the "extension"-icon over the "open in new window"-icon 👍

@kulmann
Copy link
Member

kulmann commented Nov 8, 2021

Also preferring the extension-icon.
While I agree that the solution to add the app display names for multiple languages feels a bit weird, I also don't see a better solution right now. Actually, web supports 6 languages (hardcoded) at the moment: de, fr, es, it, cs, gl. Could you please add the other ones as well? 😅

@AlexAndBear
Copy link
Contributor

@kulmann yes, for sure.
What do you expect, if there is no translation for it, is the fallback to eng legit?

@kulmann
Copy link
Member

kulmann commented Nov 8, 2021

@kulmann yes, for sure.
What do you expect, if there is no translation for it, is the fallback to eng legit?

Yep, english as fallback is perfect.

@JammingBen
Copy link
Collaborator Author

The extension icon is not yet available, is it? https://owncloud.design/#/Design%20Tokens/Icon That would need to be implemented in the ODS, I assume?

@kulmann
Copy link
Member

kulmann commented Nov 9, 2021

The extension icon is not yet available, is it? https://owncloud.design/#/Design%20Tokens/Icon That would need to be implemented in the ODS, I assume?

Oh yes, true. You can add the .svg file to the design system in a PR and we'll release it quickly after. There is an ODS release in the pipeline for today or tomorrow anyway.

@AlexAndBear
Copy link
Contributor

👍

@ownclouders
Copy link
Contributor

Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/20144/14/1
The following scenarios passed on retry:

  • webUISharingAcceptSharesToRoot/acceptShares.feature:108

@AlexAndBear AlexAndBear added the Status:Needs-Review Needs review from a maintainer label Nov 9, 2021
@pascalwengerter
Copy link
Contributor

@JammingBen @janackermann new (extension) icon is on current master with the new ODS release :)

@JammingBen
Copy link
Collaborator Author

Great, thx! :)

This is now ready for review, @kulmann maybe?

@pascalwengerter pascalwengerter mentioned this pull request Nov 11, 2021
26 tasks
Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

Just tried to get this to work (still would like @kulmann's opinion on the PHP parts 😅 ), seems like the extension icon itself doesn't render in the web context (but does on https://owncloud.design/#/Design%20Tokens/Icon. I'll investigate and perhaps replace it in a bugfix ODS release later today to get this merged in the release!

@pascalwengerter
Copy link
Contributor

pascalwengerter commented Nov 11, 2021

Just tried to get this to work (still would like @kulmann's opinion on the PHP parts sweat_smile ), seems like the extension icon itself doesn't render in the web context (but does on https://owncloud.design/#/Design%20Tokens/Icon. I'll investigate and perhaps replace it in a bugfix ODS release later today to get this merged in the release!

Caught it, ODS bugfix release incoming part of this PR

extension-oc10

@ownclouders
Copy link
Contributor

Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/20243/14/1
The following scenarios passed on retry:

  • webUISharingAcceptSharesToRoot/acceptShares.feature:108

Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

I updated changelog and docs. Code looks good. Also checked it in an app deployment and works like a charm. 😍

@sonarcloud
Copy link

sonarcloud bot commented Nov 11, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@ownclouders
Copy link
Contributor

Results for oC10Files1 https://drone.owncloud.com/owncloud/web/20250/15/1
The following scenarios passed on retry:

  • webUIFilesActionMenu/versions.feature:15

@kulmann kulmann merged commit 9fa2836 into master Nov 11, 2021
ownclouders pushed a commit that referenced this pull request Nov 11, 2021
Merge: 8a0f765 4ff8957
Author: Benedikt Kulmann <benedikt@kulmann.biz>
Date:   Thu Nov 11 16:43:20 2021 +0100

    Merge pull request #5996 from /issues/5980

    Automatically show oC10 apps in app switcher menu
@pascalwengerter pascalwengerter deleted the issues/5980 branch November 11, 2021 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform:oC10 Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show oC 10 Apps automatically in Web UI App Switcher
6 participants