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

Settings not saved when changing tray icon theme #456

Closed
3 tasks done
CromFr opened this issue Feb 27, 2017 · 6 comments
Closed
3 tasks done

Settings not saved when changing tray icon theme #456

CromFr opened this issue Feb 27, 2017 · 6 comments

Comments

@CromFr
Copy link

CromFr commented Feb 27, 2017

I confirm (by marking "x" in the [ ] below):


Summary
Settings not saved when changing tray icon theme

Steps to reproduce

  • Operating System: Archlinux
  • Mattermost Desktop App version 3.6.0
  • Mattermost Server version: 3.6.0 (3.6.2)
  • Clear steps to reproduce the issue
    • Go to file/settings
    • Try to change the tray icon theme

Expected behavior

"Settings saved" toast, and settings effective & saved (after restarting mattermost-desktop)

Observed behavior

Tray icon theme is still on the last value set.
Also, if you change another setting after changing the tray icon, everything works as expected

Possible fixes

maybe the save settings callback is not called with radio buttons?


nice issue template btw :)

@jasonblais
Copy link
Contributor

@yuya-oc can you help take a look? Potentially a critical bug

@yuya-oc
Copy link
Contributor

yuya-oc commented Feb 27, 2017

@CromFr Thanks for feedback. Yeah, auto-saving is missing in callback of radio buttons as you mentioned. So we need to fix.

@jasonblais
Copy link
Contributor

To confirm, does this mean we should do 3.6.1 for Linux?

Is it only for the theme setting change?

@yuya-oc
Copy link
Contributor

yuya-oc commented Feb 28, 2017

@jasonblais Ah, I didn't think about that.

As far as I read the codes again, only theme doesn't start auto-saving. As reported in original observed behavior, fortunately the theme is saved by changing another app option. So I feel we can cut v3.6.1.

@jasonblais
Copy link
Contributor

I'm not really familiar with the Linux UI - how common is it to change the theme option?

It sounds like it might be a one-time change, for which there's a workaround for now (changing another app option). So I'm not sure it warrants a v3.6.1, although we should fix it for v3.7 of course.

@jasonblais
Copy link
Contributor

Closed by #465, thanks @CromFr and @yuya-oc!

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

No branches or pull requests

3 participants