puavo-org / externalportal

External sites portal widget for Nextcloud dashboard
GNU Affero General Public License v3.0
6 stars 2 forks source link

Feature: colored icons #3

Closed theCalcaholic closed 1 year ago

theCalcaholic commented 1 year ago

This PR is based on #2 and includes its changes!

I like your app and recently had the issue, that it doesn't play very well with 1) theming and 2) light/dark mode, because the contrast between the icons and the widget isn't guaranteed to be good. So I decided to attempt implementing options for dynamic icon colors.

Implements 4 options for icon colors:

  1. use icon as-is (the previous behavior) image
  2. use text color image
  3. use theme color image
  4. use custom color image

With options 2-4, the icon will use mask-image to dynamically color the icon with the specified color.

Mazhoon commented 1 year ago

I'm having trouble getting custom colour to work (Nextcloud 26, both Firefox and Chrome): I can open the colour picker, but after selecting a colour in the popup, the colour on the settings page always remains black. No errors in JS console - if I set the value to something else via JS console, the colour on the page gets updated correctly. However, clicking "Save" causes the colour to reset to black.

Edit: Just to confirm, removing v-model="state.customIconColor" in AdminSettings.vue:64 allows me to get another colour shown on the settings page (naturally, it doesn't get saved then), so it appears that the problem is caused by model-binding resetting the selected colour after the selected colour has changed, instead of picking up the new colour from the selection. No idea (yet?) what causes this

theCalcaholic commented 1 year ago

@Mazhoon I finally found the time to look into this. Tbh, I'm now confused how the color field was every working for me during my tests :D NCInputField is not supposed to be used like that. I have now implemented it properly. Feel free to give it a try.

Mazhoon commented 1 year ago

@Mazhoon I finally found the time to look into this. Tbh, I'm now confused how the color field was every working for me during my tests :D NCInputField is not supposed to be used like that. I have now implemented it properly. Feel free to give it a try.

Thank you very much, seems to work now! Closing this PR as I rebased these commits locally and pushed to main branch.

Also, after pushing, I noticed that the new file src/ColorInputField.vue doesn't have copyright headers yet. I'll add a SPDX-FileCopyrightText: Tobias Knöppler 6317548+thecalcaholic@users.noreply.github.com SPDX-License-Identifier: AGPL-3.0-or-later header to it before next release if that's ok by you.

theCalcaholic commented 1 year ago

Also, after pushing, I noticed that the new file src/ColorInputField.vue doesn't have copyright headers yet. I'll add a SPDX-FileCopyrightText: Tobias Knöppler 6317548+thecalcaholic@users.noreply.github.com SPDX-License-Identifier: AGPL-3.0-or-later header to it before next release if that's ok by you.

Sure, thank you!