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

dashboard: add page reload on successful save #7461

Merged
merged 4 commits into from
May 23, 2024

Conversation

Monviech
Copy link
Sponsor Member

When trying out the new dashboard, the save button didn't give me any feedback on successful save. So I have added a page reload.

@swhite2
Copy link
Member

swhite2 commented May 22, 2024

While this would certainly work with the least amount of code, the intent of the original design is to keep everything as dynamic as possible. This action will interrupt the keep-alive sessions collecting data.

I think this is fine for now, though perhaps there is a less invasive way we can think of to let the user know the action was succesful. Any opinion? :)

@Monviech
Copy link
Sponsor Member Author

Hello, we could also display a success message in a HTML message area, or make the save button vanish upon successful save, e.g. only reloading the area with the buttons?

I just replicated the behavior of the Restore Defaults button with the full reload of the whole page.

@swhite2
Copy link
Member

swhite2 commented May 22, 2024

Yes, the restore defaults was the only exception here, although in theory only the widget positions and width/height need to update with the current logic, this has a lower priority.

we could also display a success message in a HTML message area, or make the save button vanish upon successful save, e.g. only reloading the area with the buttons?

If you can spare the time to propose something in this PR that'd be great - otherwise I'll add it to my todo list and merge this one for the time being.

@Monviech
Copy link
Sponsor Member Author

Sure I can try.

…ces feedback on click. The button is unlocked and the spinner is hidden upon success or failure. On success, the button is hidden.
@Monviech
Copy link
Sponsor Member Author

@swhite2 What do you think of this idea? I think it fits to the general responsiveness of the new dashboard. Though, if it's not what you want, I'm open to change things, or pass you the torch on this. ^^

@swhite2
Copy link
Member

swhite2 commented May 23, 2024

@Monviech In terms of styling this looks really nice :)

I would invert the action and timeout, so the save action starts immediately. The spinner would be active while this happens, and after it completes add a small artificial delay (perhaps 500ms) instead (regardless of success/failure state). Users are quick and with this we can't guarantee the save action will fire if the user navigates to somewhere else within the delay period.

@Monviech
Copy link
Sponsor Member Author

Thanks for the feedback. Yeah that makes sense.

I cant continue here until next week. If you need to work on the file I suggest you merge or close the PR, and we can fix the action and timeout sequence later.

@swhite2
Copy link
Member

swhite2 commented May 23, 2024

I'll change it in this PR in a bit and merge it after. Many thanks for the initial code.

FWIW, the dashboard is under active development so if you have any other feedback or changes, all welcome :)

@swhite2 swhite2 merged commit 8c68186 into opnsense:master May 23, 2024
@swhite2
Copy link
Member

swhite2 commented May 23, 2024

@Monviech Got a bit carried away with the styling part here.. sorry about that ;)

@Monviech
Copy link
Sponsor Member Author

@swhite2 Hey it looks interesting and like you had fun. :D Im going to try it out on the weekend. Thanks ^^

@Monviech Monviech deleted the new-dashboard-reload-on-save branch June 6, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants