Toggling mode causes data reload

ananthakumaran / paisa

Paisa – Personal Finance Manager. https://paisa.fyi demo: https://demo.paisa.fyi

https://paisa.fyi

GNU Affero General Public License v3.0

2.5k stars 131 forks source link

Toggling mode causes data reload #37

Closed vomaksh closed 1 year ago

vomaksh commented 1 year ago

Describe your Environment OS: Windows 11 Paisa Version: 0.5.1 App Variant: Desktop

Describe the bug Changing dark mode to light mode or vice versa reloads data

Expected behavior Mode should change but data shouldn't be reloaded IMHO.

ananthakumaran commented 1 year ago

Have you tried the latest master? I have put a fix for this issue, also reported by https://github.com/ananthakumaran/paisa/issues/26

I know reloading the page is not ideal and I indeed started with CSS change only. There are a few places where I set the color from javascript (SVGs) and just changing at the CSS level would leave them in an ugly state, so I decided to make this tradeoff (vs handling it all over the code). But you shouldn't lose any data because of this. Let me know if there are any cases where you can lose data due to this.

vomaksh commented 1 year ago

I have tested this on demo. I am guessing that it is latest.

ananthakumaran commented 1 year ago

Just to clarify, the page does reload when you toggle the switch. This is known, but on the editor page, if you have unsaved changes, then you should get a popup asking for confirmation. Let me know if you don't get that popup. Please record and post a GIF if that's the case. I have tested it and not able to reproduce the issue

vomaksh commented 1 year ago

There are some issues I found on the editor page:-

Clicking on cancel button in the confirmation box changes the mode but doesn't reload If one clicks cancel then one expects that no changes will take place but theme color changes but the editor doesn't reload. IMHO it is inconsistent.
I think confirmation popup is a bad idea Many browsers disable by default or give the option to disable popup if they show frequently. I think you should have a custom dialog component.

In the end I am adding a small video about the above things I mentioned.

https://github.com/ananthakumaran/paisa/assets/26306108/6756e81c-a07e-4647-a418-c873332a8466

ananthakumaran commented 1 year ago

Thanks for the screen recording

but theme color changes but the editor doesn't reload

The color theme change is a minor bug, I have fixed it on master (can be verified on demo).

I think confirmation popup is a bad idea Many browsers disable by default or give the option to disable popup if they show frequently. I think you should have a custom dialog component.

Agree it's not a good experience, but the way the browser works, I don't have much choice. When a user clicks the back button, the website is not allowed to block it and show a dialog, the only option available is what I am currently doing. For this specific case, it's possible to show a dialog, but then I have to handle things differently for this vs. other navigation. I have decided to keep things simple for now, I might revisit the decision later.