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

"Show Clickable Link" checkbox does not update until entire dashboard is saved #3794

Open
2 tasks done
BBaoVanC opened this issue Sep 24, 2023 · 10 comments
Open
2 tasks done
Labels
area:dashboard The main dashboard page where monitors' status are shown bug Something isn't working good first issue Good for newcomers

Comments

@BBaoVanC
Copy link

⚠️ Please verify that this bug has NOT been raised before.

  • I checked and didn't find similar issue

🛡️ Security Policy

Description

After checking the "Show Clickable Link" box in a monitor's setting dialog and closing the dialog, reopening the monitor's setting dialog shows the same box still unchecked giving the impression that the setting did not change. Only after saving aving the dashboard and returning to the monitor's setting dialog does the box stay checked.

👟 Reproduction steps

  1. Press Edit Status Page
  2. Press the settings cog on a monitor
  3. Tick "Show Clickable Link" and close the dialog
  4. Press settings cog of the same monitor again

👀 Expected behavior

The checkbox should stay checked to hint that it will be enabled after pressing save on the entire status page

😓 Actual Behavior

The checkbox remains unchecked and the setting seemingly didn't change

🐻 Uptime-Kuma Version

1.23.2

💻 Operating System and Arch

Debian 11

🌐 Browser

Firefox 117.0.1

🐋 Docker Version

Docker 24.0.6

🟩 NodeJS Version

No response

📝 Relevant log output

No response

@Rai-Sahil
Copy link

Can I work on it? If you think it's could be good first issue.

@CommanderStorm
Copy link
Collaborator

You can work on this.
We don't assign people to issues ^^

See our contribution guide: https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md

@Rai-Sahil
Copy link

Hi @BBaoVanC

How do you recreate the bug? I tried the edit stats page it doesn't really show this checkbox.

@BBaoVanC
Copy link
Author

Hi @BBaoVanC

How do you recreate the bug? I tried the edit stats page it doesn't really show this checkbox.

I sent a picture with the steps of reproducing it in a different thread: #3795 (comment). If it's not there, what version are you on?

@Rai-Sahil
Copy link

Rai-Sahil commented Nov 14, 2023

Oh @BBaoVanC

found it. thanks

Also I fixed the issue, should I just make a pull request now?

Screen.Recording.2023-11-13.at.10.11.45.PM.mov

Like this is my first PR, so just wondering,

  • Fork the repo
  • Make a new branch named "Issue###Fixed"
  • then make a PR for my branch (from my forked repo) and the main branch of the main repo.

right?

@Rai-Sahil
Copy link

You can work on this. We don't assign people to issues ^^

See our contribution guide: https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md

Hi @CommanderStorm

I have made a PR. you can see from recording I included in the PR that the issue is fixed. All the tests from auto-check to ESLint. Everything is good.
Screenshot 2023-11-13 at 11 38 46 PM

I was wondering if you can merge that. Would be really helpful thanks.

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Nov 14, 2023

@Rai-Sahil
Please adress the comment from nelson in the PR.

As I wrote to you via email (sending messages via multiple channels at the same time is not good practice, as it just annoys people ^^):

I am not fully convinced that saving a single value to local storage fixes the issue as this means that

  • either all checkboxes are set or not
  • or maybe even requires a reload of the page to get somewhere

@Rai-Sahil
Copy link

@CommanderStorm

If you want I can change it from the local storage to something else. From the code, I saw that you guys are saving the current state of checkbox in a variable. Even I'm doing the same thing but just storing the value in local storage.

Although I tried refreshing the page, I saw no checkbox other than the intended one was checked or not. But I get your point that should be saved in user state. Not that if user closed the browser and re-opens it there. If you want I can come up with some other way as well.

@CommanderStorm CommanderStorm added area:dashboard The main dashboard page where monitors' status are shown good first issue Good for newcomers labels Dec 8, 2023
@kevinmatthe
Copy link

It seems that this issue is still unfixed, was there a temporary solution to make it take effect?

@CommanderStorm
Copy link
Collaborator

was there a temporary solution to make it take effect?

Once the entire dashboard is saved, the checkbox updates.
If you'd like to contribute a fix for this, here is our contribution guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dashboard The main dashboard page where monitors' status are shown bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants