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 search providers icons #41432

Merged
merged 2 commits into from Nov 14, 2023
Merged

Fix search providers icons #41432

merged 2 commits into from Nov 14, 2023

Conversation

Altahrim
Copy link
Collaborator

@Altahrim Altahrim commented Nov 13, 2023

Related to advanced search

  • Use white icon for user search provider
  • Add missing icons for settings search providers

@Altahrim Altahrim added the 3. to review Waiting for reviews label Nov 13, 2023
@Altahrim Altahrim added this to the Nextcloud 28 milestone Nov 13, 2023
@Altahrim Altahrim self-assigned this Nov 13, 2023
@Altahrim Altahrim changed the title Fix user search provider icon Fix search providers icons Nov 13, 2023
@Altahrim Altahrim requested review from a team, ArtificialOwl, icewind1991, sorbaugh, Fenn-CS and emoral435 and removed request for a team November 14, 2023 08:35
@Altahrim
Copy link
Collaborator Author

CI failure unrelated

@Altahrim Altahrim mentioned this pull request Nov 14, 2023
@@ -221,6 +221,10 @@ private function fetchIcon(string $appId, string $providerId): string {
[$appId, 'app.svg'],
['core', 'places/default-app-icon.svg'],
];
if ($appId === 'settings' && $providerId === 'users') {
// The file /apps/settings/users.svg if already used in black version by user menu
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// The file /apps/settings/users.svg if already used in black version by user menu
// The file /apps/settings/users.svg is already used in black version by user menu

Typo maybe? Or I do not understand the comment.
But still I do not understand the comment I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, typo…
We are using /apps/settings/users.svg as an icon in the user menu on top right. In the menu, we need a black icon.
But for search, we need a white version.

Copy link
Contributor

@Fenn-CS Fenn-CS Nov 14, 2023

Choose a reason for hiding this comment

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

Must these icons have the same name? Can't both colors be present? to avoid this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Must these icons have the same name?

My hack avoid this. The new icon is white and we keep the old one

Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
@Altahrim Altahrim merged commit e90df64 into master Nov 14, 2023
46 of 48 checks passed
@Altahrim Altahrim deleted the fix/white-user-icon-search branch November 14, 2023 17:09
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
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants