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

Compatibility with 15 #421

Closed
MorrisJobke opened this issue Nov 15, 2018 · 9 comments
Closed

Compatibility with 15 #421

MorrisJobke opened this issue Nov 15, 2018 · 9 comments

Comments

@MorrisJobke
Copy link
Member

MorrisJobke commented Nov 15, 2018

It would be nice to get this available for Nextcloud 15. There is a list of breaking changes in nextcloud/server#11045, but I doubt that the polls app has problems with that. It should be limited to raising the max version number in info.xml, give it a test and then publish a new version to the app store.

cc @dartcafe

@dartcafe
Copy link
Collaborator

I checked NC 15 already and it seems to work exept generating icons from the app.
NC 15:
grafik

NC14
grafik

@dartcafe
Copy link
Collaborator

dartcafe commented Nov 15, 2018

polls/css/colors.scss

Lines 12 to 28 in 681ce74

// Icon definitions
@mixin icon-color($icon, $dir, $color, $version: 1, $core: false)
.icon-app {
@include icon-color('app','polls',$color-text-maxcontrast)
}
.icon-yes {
@include icon-color('checkmark','actions',$fg-yes,1,true)
}
.icon-comment-yes {
@include icon-color('comment','actions',$fg-yes,1,true)
}
.icon-comment-no {
@include icon-color('comment','actions',$fg-no,1,true)
}
.icon-no {
@include icon-color('close','actions',$fg-no,1,true)
}

i.e. results in
--icon-no: url('/index.php/svg/core/actions/close/f45573?v=1');
and

--icon-close-f45573: url(data:image/svg+xml;base64,PHN...);

I expected
--icon-no: url(data:image/svg+xml;base64,PHN...);

@MorrisJobke
Copy link
Member Author

cc @juliushaertl

@juliusknorr
Copy link
Member

@dartcafe It seems that the --icon-no is hardcoded in vote.scss, while the colors.scss file is not even included in the poll page.

Not sure where --icon-no: url('/index.php/svg/core/actions/close/f45573?v=1'); is coming from, I didn't get this when trying.

@dartcafe
Copy link
Collaborator

???

@import 'colors.scss';

@juliusknorr
Copy link
Member

@dartcafe Ok I probably was confusing something then. Anyway is there a reason you have a separate .no class in the vote.scss file? If you just use the .icon-no/.icon-yes classes you define in colors.scss for your element it works fine.

image

@dartcafe
Copy link
Collaborator

No special reason, but somehow I found the creation of css variables useful and it worked in NC14 as described in https://docs.nextcloud.com/server/14/developer_manual/design/css.html. In NC15 the generation of the variables oviously changed, so I assumed a bug. I wanted to care later about it.

So i have to change back to the straight class definiton.

@juliusknorr
Copy link
Member

Yes, sorry about the docs there, the icon variables should probably not being considered as stable (aka public api) cc @skjnldsv We should indicate that devs should just use the mixins.

@skjnldsv
Copy link
Member

@juliushaertl yes! 😉 there is a lot of things we need to put on the docs 😁

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

No branches or pull requests

4 participants