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

Feat: Server Default Appearances #4478

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chakflying
Copy link
Collaborator

@chakflying chakflying commented Feb 11, 2024

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Fixes #1858

Implement server default appearances by injecting JSON data. If we use the established way of loading data after the front-end is initialized, there would be a flash of the default settings, which would not be a good user experience. Hence the method of preloading data for the status pages is also used to preload the server default settings into the HTML response.

Unfortunately this does not seem to play nice with the status page implementation, but there should be no reason for both middleware to exist at the same time. Hence the middleware is applied AFTER the status page routers are defined.

Type of change

  • User interface (UI)
  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
  • My changes generates no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

image

@chakflying chakflying added area:settings Related to Settings page and application configration area:ui/ux Interface and User Experience issues labels Feb 11, 2024
@CommanderStorm
Copy link
Collaborator

Just a bit of quick feedback (you are likely aready aware of this):
If I were a user and were presented with this UI I would not know what do here..

"What is a Appereance > Server?"

Semi-related meme

image

@chakflying
Copy link
Collaborator Author

Yes, I'm also not convinced that Browser/Server is the right word to use. Open to feedback.

@louislam
Copy link
Owner

I think it will be more clear and easier to implement if it can be chosen either one only.

My suggestion would be adding a dropdown Appearance Mode:

  • Sync with server = The styles be stored on server, all browsers must respect this styles
  • Session based = current mode

@CommanderStorm CommanderStorm added the needs:work this PR needs work label May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:settings Related to Settings page and application configration area:ui/ux Interface and User Experience issues needs:work this PR needs work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Appearance settings lost after browser cache clear
3 participants