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

Fixes #4781: Adds Edit + Add Page ScrollToTopPage() On Settings Tab Form Error Messages #4782

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

thabaum
Copy link
Contributor

@thabaum thabaum commented Oct 24, 2024

Fixes #4781:

  • Adds Edit + Add page ScrollToTopPage() on error messages.

@sbwalker
Copy link
Member

@thabaum the page name validation is already handled in the existing code:

There is a required attribute on the field:

<input id="name" class="form-control" @bind="@_name" maxlength="50" required />

and a form validator in the Save method:

if (await interop.FormValid(form))

This handles all of the required fields.

@thabaum
Copy link
Contributor Author

thabaum commented Oct 24, 2024

@sbwalker I guess I was thinking about this error message

'
AddModuleMessage(Localizer["Message.Required.PageInfo"], MessageType.Warning);
if (string.IsNullOrEmpty(_name))
{
AddModuleMessage(Localizer["Message.Required.PageName"], MessageType.Warning);
}
await ScrollToPageTop();
'

Just seemed more friendly to have the error message tell what is the issue instead of looking for a red x in the form for the blank page which I think is the only one you can really mess up beside the effective date on the form.
"You Must Provide Page Name, Theme, and Container"

This message however does not get fired off when the page is blank.

Instead you get this one:
AddModuleMessage(SharedLocalizer["Message.InfoRequired"], MessageType.Warning);
"Please Provide All Required Information"

A more generic message for this error which I understand is probably preffered.

I can remove the message enhancements I suppose it is a bit more simple to keep it just a message that makes a user review the entire form.

@thabaum thabaum changed the title Fixes #4781: Adds Edit + Add page ScrollToTopPage() on error messages + Page Name Error Message Fixes #4781: Adds Edit + Add page ScrollToTopPage() on error messages Oct 24, 2024
@thabaum
Copy link
Contributor Author

thabaum commented Oct 24, 2024

@sbwalker I went ahead and removed the added error message for blank page name. This just focuses on scroll to top of page functionality on error messages.

@thabaum thabaum changed the title Fixes #4781: Adds Edit + Add page ScrollToTopPage() on error messages Fixes #4781: Adds Edit + Add Page ScrollToTopPage() On Form Error Messages Oct 24, 2024
@thabaum thabaum changed the title Fixes #4781: Adds Edit + Add Page ScrollToTopPage() On Form Error Messages Fixes #4781: Adds Edit + Add Page ScrollToTopPage() On Settings Tab Form Error Messages Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH] Page Management: Scroll To Top Of Page on Error for Edit and Add Page
2 participants