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(theming): Stop leaking user theme into capabilities #40877

Closed

Conversation

provokateurin
Copy link
Member

Summary

The capabilities were mixing the admin theme and the user theme. For now I set to only use the admin theming, but I intend to submit a second fix that uses the user theme if a the request came from a logged in user.

Checklist

@juliusknorr
Copy link
Member

Might need some alignment between @nextcloud/designers and @tobiasKaminsky as those capabilities are used to style the clients. I would still expect the user colors in there

@provokateurin
Copy link
Member Author

I agree that the user theming should be exposed in the capabilities. I intend to implement that, but currently the capabilities expose a mix of user and admin theming.

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.

I agree that the user theming should be exposed in the capabilities. I intend to implement that, but currently the capabilities expose a mix of user and admin theming.

If this will be done in a follow-up, I'd say this PR is fine from design perspective 👍

@tobiasKaminsky
Copy link
Member

Right now I see no change for clients.
If this is true, I am fine with it :)

@susnux
Copy link
Contributor

susnux commented Oct 19, 2023

@provokateurin Drone errors are related.
I think you have to fix apps/theming/tests/CapabilitiesTest.php

@provokateurin
Copy link
Member Author

Yes I know, I just didn't have the time yet :)

@provokateurin provokateurin force-pushed the fix/theming/capabilities-user-theme-leak branch from a11a4f7 to 8a844c8 Compare October 23, 2023 06:49
@provokateurin provokateurin force-pushed the fix/theming/capabilities-user-theme-leak branch from 8a844c8 to 837eb45 Compare October 23, 2023 07:30
@provokateurin
Copy link
Member Author

I think the drone failure now is unrelated. Can someone confirm and possibly give it a force merge?

@provokateurin
Copy link
Member Author

Closed in favor of #41059

@skjnldsv skjnldsv deleted the fix/theming/capabilities-user-theme-leak branch March 14, 2024 07:51
@skjnldsv skjnldsv removed this from the Nextcloud 28 milestone Aug 14, 2024
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.

6 participants