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

visual issues if using other themes #596

Closed
Lyterio opened this issue Nov 13, 2018 · 6 comments
Closed

visual issues if using other themes #596

Lyterio opened this issue Nov 13, 2018 · 6 comments

Comments

@Lyterio
Copy link

Lyterio commented Nov 13, 2018

Steps to reproduce

  1. Install and activate your own Theme (for example https://github.com/mwalbeck/nextcloud-breeze-dark) in your nextcloud
  2. navigate to bookmarks app
  3. click on a bookmark to open details

Expected behaviour

The "bookmark-detail slide-in" and dropdown list (select2) should look like the main theme. See files app. This has the same detail slide in and also tags and dropdown.

I would expect that the bookmark app would use existing css classes from nextcloud instead of creating own styles.

Actual behaviour

Bookmark app has its own css styles even for styles which could be used from nextcloud for example select2.css. Why not use nextcloud css to get unified look and feel? Also makes it hard for creating own themes because of additional / redundant css.

Server configuration

Nextcloud version: 14.0.3
Bookmarks version: 0.14.2

@marcelklehr
Copy link
Member

Bookmark app has its own css styles even for styles which could be used from nextcloud for example select2.css.

That's true. The reason for this is that the bookmarks app comes with its own version of select2 which is incompatible with the one used by nextcloud.

As far as I know, the sidebar hasn't been standardized yet. See https://docs.nextcloud.com/server/14/developer_manual/design/index.html for example, where it doesn't show up. Thus I don't think the css selectors used in the files app are treated as a public API, which would make using them very problematic. I'm always open for suggestions or corrections, though.

@Lyterio
Copy link
Author

Lyterio commented Nov 14, 2018

Hi marcelklehr,

That's true. The reason for this is that the bookmarks app comes with its own version of select2 which is incompatible with the one used by nextcloud.

They look similar and do the same. In which way are they incompatible? Even if the functionality were different at least the css styles could be used from nextcloud.

I don't think this has to be described in the dev manual. The guidelines can't cover 100% anyways.

Reusing existing code is common sense and should be best practice. So if there is the same functionality in one of nc's core apps (files with sidebar and tags) this should be reused as much as possible.

Check the developer manual css. Creating your own theme can be hard if apps have their own css classes or elements. Reinventing something does increase the spent time and maintenance.

Just wanted to inform that this can be improved. It's up to you weather to do it or not.
Thanks for this great app!

@marcelklehr
Copy link
Member

That's true. The reason for this is that the bookmarks app comes with its own version of select2 which is incompatible with the one used by nextcloud.

They look similar and do the same. In which way are they incompatible? Even if the functionality were different at least the css styles could be used from nextcloud.

They are incompatible on an API consumer level, which means it is not trvial to replace one with the other, but it can be done.

Reusing existing code is indeed a good practice, I agree with you, however, software is bound to change. With that comes the responsibility of declaring the likelihood of changes on specific parts and the caution to not use parts directly that weren't declared for public usage, because they are likely to change. While it is often convenient to reuse code it is not always future-proof to do so.

If the bookmarks app used the styles of the files app, it would depend on them staying exactly as they are now in future releases, but as they aren't even documented this is not likely in my opinion.

Thank you for taking the time to provide this feedback. Creating a theme for nextcloud at present is indeed a hard task, in my opinion. Precisely because lots of things are not unified and can't be unless they are standardized. I'll leave this issue open and get back to this, once the sidebar standardization is done.

@Lyterio
Copy link
Author

Lyterio commented Nov 15, 2018

Missing standardizations is already known and someone seems to be working on it:
Standardisation summary #8374
Sidebar standardization #10289
There is hope! 😄

@mat-m
Copy link

mat-m commented Dec 24, 2018

Bookmarks is not mentioned in #8374.

Does someone plans to fix values with respect to vars, at least ?

@marcelklehr
Copy link
Member

This shoud be resolved with the upcoming v2.1.0 -- kindly shout or open a new issue, if there are still obstacles left. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants