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

Switching form for homepage without reloading page #786

Merged
merged 3 commits into from
Dec 3, 2024
Merged

Conversation

sfisher
Copy link
Contributor

@sfisher sfisher commented Nov 9, 2024

This is a continuation of #541 .

This makes it so that switching to ARK or DOI changes the form below without reloading the page.

It also includes some of the previous fixes for VPAT stuff. If you accept that previous PR first then the diff will only show the things actually changed for this PR.

This now moves the form to a partial. It adds some actions controller actions for handling AJAX. It adds javascript for triggering the AJAX calls and then re-renders the new content into the appropriate div for the new form.

I also had to add some hidden fields and leave the radio buttons for the ARk/DOI outside the main form since it will get erased when the form is re-rendered so that it needs to stay outside the main form.

I took a while to figure out why the normal one didn't work and apparently the layout for it doesn't work with fieldset but needs to stay as a div instead.

We can talk through this in our dev sync or other things if you like since it may not be obvious how it works if you're not familiar with AJAX and some of the Javascript.

@sfisher sfisher requested a review from jsjiang November 9, 2024 00:46
@sfisher sfisher changed the base branch from main to develop November 12, 2024 17:46
@sfisher sfisher changed the base branch from develop to main November 12, 2024 22:54
@sfisher sfisher changed the base branch from main to develop November 12, 2024 22:55
Copy link
Contributor

@jsjiang jsjiang left a comment

Choose a reason for hiding this comment

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

@sfisher Thank you Scott for explaining the code change to me.
How about we review the ajax_index_form approach with the UX team to make sure it meets the accessibility standards.

Jing

@sfisher
Copy link
Contributor Author

sfisher commented Nov 15, 2024

Sure, if you like. Their problem with the old form was that the page reloading resets the user/screen reader place on the page (on the ark/doi radio buttons). This addresses it and is a fairly standard alternative to reloading the whole page so that the place on the page isn't reset by a page reload.

But I'm sure we can run it by John Kratz or someone on the team and they'd be happy to look at it.

@sfisher
Copy link
Contributor Author

sfisher commented Nov 15, 2024

We're not currently using this, though may be useful in the future. If we come back to this branch it will be here.

@sfisher sfisher closed this Nov 15, 2024
@sfisher
Copy link
Contributor Author

sfisher commented Nov 20, 2024

Ooopsie. I think I accidentally closed the wrong ticket here and it wasn't merged. So re-opening.

@sfisher sfisher reopened this Nov 20, 2024
Copy link
Contributor

@jsjiang jsjiang left a comment

Choose a reason for hiding this comment

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

@sfisher Looks good. Thank you Scott.

Jing

@jsjiang jsjiang merged commit 66f8a11 into develop Dec 3, 2024
1 check passed
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.

2 participants