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

[stable25] New theming, how to make your app compatible #32060

Closed
skjnldsv opened this issue Apr 22, 2022 · 17 comments
Closed

[stable25] New theming, how to make your app compatible #32060

skjnldsv opened this issue Apr 22, 2022 · 17 comments
Labels
25-feedback design Design, UI, UX, etc. enhancement feature: theming overview pending documentation This pull request needs an associated documentation update
Milestone

Comments

@skjnldsv
Copy link
Member

skjnldsv commented Apr 22, 2022

With #31751 we decided to drop scss variables.

Not everything is compatible with css variables and you can not do colors computation anymore
Prior to 25, the theming app was providing an imperative approach with a set of css rules hacks to mitigate those dark/bright discrepancies.

With 25, we now moved to a declarative approach. There are multiple things you can/have to consider when designing an app to make sure you have a minimalistic work to do to fulfill said compatibility

  1. If your app is made with vue, migrate away from icon-class to vue material design icons https://www.npmjs.com/package/vue-material-design-icons
  2. If you're using icon-classes
    1. Plan your app to look proper with the default theme.
      That means the icons should be dark if they are over the main background and white if they are over the primary background colour
    2. Make sure to use dedicated div/span for the icon only
    3. And finally, use the following css rule on said icon element
      --primary-invert-if-bright if the icon is over the primary background colour
      --background-invert-if-dark if the icon is over the background colour
      This will automatically invert the icon colour if necessary.
  3. Do not use body[theme-xxx] selector to create custom rules. We now also use media queries (1, 2) to match the browser preferences. That means you have to let the Theming do its job and not try to do it yourself

Removal of SVG colour api

With 25 we remove the support of the icon-color and icon-black-white scss mixins.
We move to a static generated icons colour approach #31918

While we still ask you to migrate to inline svg that automatically reflect the font colour and are compatible with any themes (like the vue material design icons), if you wtill need to use the old icons classes, you can keep doing so (until removal in the future)

  • You can find the list of classes in the icons.css file and the source file
  • Each class is linked to an icon with css variable, this variable is automatically inverted from dark to white and vice-versa if one of the activated theme is a dark theme.
  • You can use the css class like .icon-more like before or also use the css variable straight away:
    • background-image: var(--icon-more-dark); or background-image: var(--icon-more-white);
      for automatic colour invert
    • background-image: var(--original-icon-more-white);
      to force a specific colour
@skjnldsv skjnldsv added enhancement design Design, UI, UX, etc. feature: theming overview pending documentation This pull request needs an associated documentation update labels Apr 22, 2022
@skjnldsv skjnldsv added this to the Nextcloud 25 milestone Apr 22, 2022
@szaimen
Copy link
Contributor

szaimen commented Apr 22, 2022

This should be planned well. Otherwise it will slip through.

@skjnldsv
Copy link
Member Author

This is still a wip, I also have another idea regarding icons, so we'll have to check if it works.

@szaimen
Copy link
Contributor

szaimen commented Apr 22, 2022

Okay

@ankit715

This comment was marked as spam.

@Rello
Copy link
Contributor

Rello commented Aug 30, 2022

@skjnldsv as I have limited time to support my Analytics & Audioplayer app, I am looking for a compromise to support as many NC versions as possible.

the usage of the following approach seems a reasonable compromise for me.

--primary-invert-if-bright
--background-invert-if-dark

Is it possible to backport just these variables to NC24/23 also? then I could use one css to support both NC versions.
Its coming from the theming app, if I am not wrong. possible to think about this?

@dartcafe
Copy link
Contributor

@Rello In polls I adopted the declaration of the new variables to keep compatibility to prior nc versions.
If it helps: nextcloud/polls#2559

@skjnldsv
Copy link
Member Author

You can use css variables fallback https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties#custom_property_fallback_values

@nursoda
Copy link

nursoda commented Sep 5, 2022

I don't understand what you mean by "let the Theming do its job and not try to do it yourself" in terms of the app dev requirement to support IProvidesIcons and its methods to get dedicated light/dark icons. In my app, it was set like this before:

        public function getLightIcon(): String {
                return $this->urlGenerator->imagePath(Application::APP_NAME, 'app-light.svg');
        }
        public function getDarkIcon(): String {
                return $this->urlGenerator->imagePath(Application::APP_NAME, 'app-dark.svg');
        }

Shouldn't this be replaced by a declarative approach, too?

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 6, 2022

Well, apps icons are a bit more tricky, because they cannot be themed (thus the primary-invert-if-bright variables), so you can still provide the app icons variants.

What I meant is that you should generally just use css variables as a way to themed your app, then it will automatically works and adapt to the other themes. Don't try to design your app for dark mode, just do the light mode and use the server css variables. The rest will be managed by the theming app and make your app compatible ith all the other themes automatically.

@blizzz
Copy link
Member

blizzz commented Oct 19, 2022

@skjnldsv, this should rather be in dev docs, no?

@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 25.0.1 Oct 19, 2022
@blizzz
Copy link
Member

blizzz commented Oct 19, 2022

moving to milestons 25.0.1, because i don't know what else to do with the milestone…

@akhil1508
Copy link
Contributor

@skjnldsv So for custom theme compatibility when moving to NC 25, the idea would be to implement the ITheme interface and set the theme value in config.php?

@skjnldsv
Copy link
Member Author

@akhil1508 we do not support custom themes via ITheme (yet ?) :)
What are you trying to achieve? 😉

@akhil1508
Copy link
Contributor

What are you trying to achieve?

@skjnldsv I mainly want to apply some custom styles(globally) and some app icons. Also, want to change the main colors as defined in scss variables. It used to work in Nextcloud 24.

It does say in settings/admin/theming:

You are already using a custom theme. Theming app settings might be overwritten by that.

@skjnldsv
Copy link
Member Author

if you are trying to only create a custom theme, you can adjust the css variables and have your theme as a simple app that adds its own css.
You can have a look at what the pride app does for example https://github.com/skjnldsv/pride

regarding custom icons, depending on what you do, this might still require the use of the old legacy theming folder mechanism

@akhil1508
Copy link
Contributor

@skjnldsv Another question about customizing the look and feel. I'd like to apply styles through an app or a custom theme that can override an icon in some app. Maybe the notifications icon or the settings icon in the calendar app.

Earlier, I could override a class like icon-notifications easily enough. Or there was this xlink:href trick in the header navbar app icons.

What is the procedure(if any) to customize icons now with all these inlined svgs?

@skjnldsv
Copy link
Member Author

skjnldsv commented Mar 31, 2023

You cannot. This is a decision we took as the icons we are now using are very well researched and provide great readability. They are also directly compatible with our user themes.

Closing now as this ticket is resolved now

Oliv4945 added a commit to Oliv4945/ldapcontacts that referenced this issue May 15, 2023
As of NC25 application has to provide their own icons, see [here](nextcloud/server#32060)
The switch to NcTextField allows easy to use their wrapper for the same result than previous code
Use the MDI AccountSearchIcon which feels more appropriate than a simple magnifier
Oliv4945 added a commit to Oliv4945/ldapcontacts that referenced this issue May 15, 2023
…lity

As of NC25 application has to provide their own icons, see [here](nextcloud/server#32060)
The switch to NcTextField allows easy to use their wrapper for the same result than previous code
Use the MDI AccountSearchIcon which feels more appropriate than a simple magnifier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
25-feedback design Design, UI, UX, etc. enhancement feature: theming overview pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

No branches or pull requests

8 participants