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

Fix 1469 save button doesnt always appear to work #1488

Closed

Conversation

TimLovellSmith
Copy link
Member

Fixing #1469 by capturing errors from server side validation and presenting them to the user.

@TimLovellSmith
Copy link
Member Author

Rebasing on latest qa.

// ---------------------------
// The form could not be saved: The Description (a longer description) field is required.
// ---------------------------
var fixedStr = error.ErrorMessage;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels really hacky, but I guess it'll do. There are unhandled edge cases like there not being a ) in the string, but I can't see that happening. I probably would have done a regex replacement.

@jeffhandley
Copy link
Member

Here's an alternate proposal to fixing this problem...

When the user clicks a link to navigate to a different version, instead of doing the whole AJAX dance, we could just submit the form and include a field in the form for the desired next page. Then the existing POST action for the form would kick in and server-side validation would happen just like the user hit the Save button. If there are errors, the page would be rendered with them. If there aren't any errors, then instead of having the action redirect the user back to the package details page, it would redirect to the desired next page.

Clay Compton and others added 2 commits August 22, 2013 14:58
…ome other issues such as flickering visible message about 'you are about to navigate away' and issue where dialog is still visible after being 'dismissed' when you use back button to return to the edit page.
@TimLovellSmith
Copy link
Member Author

Closing this and submitting new fix against staging.

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.

None yet

3 participants