A failed Create call now switches to the page with an error#177
Conversation
|
That's actually really nice! I'm on holiday right now but once I get back I'll have a full look through this and merge it! |
KSJaay
left a comment
KSJaay
left a comment
There was a problem hiding this comment.
Left some comments for myself to make a few improvements in the future. Overall an amazing update and definitely something we need to improve on.
I might actually overhaul the whole system to add support for this, along with adding support in status pages as well.
| className={`monitor-configure-page-button | ||
| ${page.id === pageId ? 'active' : ''} | ||
| ${errorPages.has(page.id) ? 'error-circle' : ''}`} | ||
|
|
There was a problem hiding this comment.
Move this to classnames to make it a bit easier to read
| &.error-circle::after { | ||
| content: ""; | ||
| position: absolute; | ||
| top: 40%; | ||
| right: 10px; | ||
| width: 10px; | ||
| height: 10px; | ||
| background-color: red; | ||
| border-radius: 50%; | ||
| } |
There was a problem hiding this comment.
Not now but moving this to an explanation mark or some sort of icon in the future
| const associatedPage: Record<string, string> = { | ||
| 'name': "basic", | ||
| 'type': "basic", | ||
| 'url': "basic", | ||
| 'port': "basic", | ||
| 'icon': "basic", | ||
| 'method': "basic", | ||
| 'json_query': "basic", | ||
| 'interval': "interval", | ||
| 'retry': "interval", | ||
| 'retryInterval': "interval", | ||
| 'requestTimeout': "interval", | ||
| 'notificationType': "notification", | ||
| 'headers': "advanced", | ||
| 'body': "advanced", | ||
| 'valid_status_codes': "advanced", | ||
| } |
There was a problem hiding this comment.
Figuring out a way to create a consistent list between all input types so this doesn't need to be updated every time a new monitor type is created.
|
Going to merge this into |
2 participants
Summary
If you selected "Create" when making a new monitor, unless you were on the page with the error it was unclear what went wrong. Now it switches to a page with an error on it and every page that has an error is marked with a red dot.
New Features
Updates
.../hooks/useMonitorForm.tsx
useMonitorForm.tsxfile I created a helper functiongetPagesWithErrorsthat returns a set with pages that have errorsuseMonitorFormtosetPageIdwhich I passed from it's parentconfigure.tsx.../monitor/configure.tsx
configure.tsxwhere it is used to add theerror-circleclass to any page with an erroronClickevent handler for each page was updated to also remove the previous error page from the error set (which subsequently removes the red dot.).../monitor/styles.scss
error-circlewas added tostyles.scssand styled to create a red dotAdditional Info
I'm not 100% convinced that it's necessary to remove the red dot when clicking away from a page with an error. At the moment the error messages that are associated with each input will stay until you "Submit" again. But this way if there are multiple pages that have an error, it is clear which pages you've already been to.
(If you prefer to keep the red dots visible until a new Submit, it just requires the
onClicklogic inconfigure.tsxto be removed.)Ideally I think the most informative way to handle errors on the forms would be to validate each separately
onBlurso the user has instant feedback. But that would be a fairly involved overhaul of the entire system. So I offer this as an option to make any errors more clear to the user.