Modify logic for preferences

leanprover-community / lean4game

Server to host lean games.

https://adam.math.hhu.de

GNU General Public License v3.0

197 stars 35 forks source link

Modify logic for preferences #181

Closed Wzixiao closed 10 months ago

Wzixiao commented 10 months ago

issues: https://github.com/leanprover-community/lean4game/issues/172

Description

I modified the original logic about preferences.

Now the mobile option will not appear on the page but will be replaced by layout.

And I added the function of saving to the browser. If it is switched to true, the user's selection will be saved, otherwise it will be deleted.

Known issues (Maybe it needs to be discussed)

I want to delete everything about mobile, and use layout entirely instead, and of course I would implicitly save the mobile device field in the preferences for the previous code without having to make too many changes(because it seems to me that their functions are somewhat duplicated.).

Demonstration diagram

joneugster commented 10 months ago

Looks good to me. What's exactly the reason for keeping mobile around? Only backwards compatibility?

I think you could go ahead and replace mobile completely with the preferences. You might also be able to get rid of the IMobileContext too.

Wzixiao commented 10 months ago

Looks good to me. What's exactly the reason for keeping mobile around? Only backwards compatibility?

I think you could go ahead and replace mobile completely with the preferences. You might also be able to get rid of the IMobileContext too.

Yes, you are right, one of my ideas is to be compatible with the previous code, otherwise there are many places in the code that will be changed.

The second idea is that since layout has auto, it is not a good suggestion to use layout in other codes, so I think mobile still exists (automatically changed by layout. Of course, maybe a get function can also be provided, so that it does not need to be changed by layout).

The third idea is that the setMobile function is not called anywhere in the entire project (after the layout is completed, and of course except the layout calling setMobile).

Please wait, I will complete it in this pr. (Don’t use IMobileContext at all).

joneugster commented 10 months ago

Great! Please ping me if you are happy with the PR!

I'm not too concerned about backwards compatibility. At this stage I would still rather drop the backwards compatibility in favour of reducing complexity of the code!

Wzixiao commented 10 months ago

I finished it.

Code change:

Delete all MobileContext and replace them with PreferenceContext(There is a mobile in PreferenceContext, making it fully compatible with previous code. And I think this code layout is still good for backwards compatibility.).
Added use_preferences.ts to /state/hooks to make the code more compact.

joneugster commented 10 months ago

Thank you very much!