cds-snc / report-a-cybercrime

Report a computer crime or scam / Signaler un crime informatique ou une fraude
https://report-a-scam.cds-snc.ca
MIT License
29 stars 14 forks source link

Update error link #2341

Closed justinr86 closed 4 years ago

justinr86 commented 4 years ago

Fixes #2340

Description

Use scrollIntoView() instead of href to scroll to element.

Any new packages installed?

N/A

Required followup work

N/A

Checklist:

justinr86 commented 4 years ago

Tested, after clicking the link in error message, the browser history didn't inserted. the back button work properly now.

What page did this happen on? I have not experienced this. - Sorry, long day.. disregard.

DianeLiu2019 commented 4 years ago

Tested, after clicking the link in error message, the browser history didn't inserted. the back button work properly now.

What page did this happen on? I have not experienced this. - Sorry, long day.. disregard.

in whoareyoureportfor and whatwasaffected page . so far , in master, if we click the link in the error message ,then click the back button, the page still in the same page , doesn't go back to the previous page, but you fixed the issue in this branch

sdzhangtao commented 4 years ago

I have a gut feeling that we better not change browser's default behavior. Most internet users are used to what typical browser are working. It's like our car's gas/brake paddle, no car dealer except Tesla want to change without a good reason.

The go to error field link are for forms that are long. If the user clicks the link and go to the error field, they might want the back button to where they were in the same page (the error summary). This is the default browser default behavior now, if we change this, user might be surprised and scrambling to find out their hardworking filled form is still there.

But if business really wants to customize this, I am fine.

justinr86 commented 4 years ago

I have a gut feeling that we better not change browser's default behavior. Most internet users are used to what typical browser are working. It's like our car's gas/brake paddle, no car dealer except Tesla want to change without a good reason.

The go to error field link are for forms that are long. If the user clicks the link and go to the error field, they might want the back button to where they were in the same page (the error summary). This is the default browser default behavior now, if we change this, user might be surprised and scrambling to find out their hardworking filled form is still there.

But if business really wants to customize this, I am fine.

The problem is that the Back Button component uses the browser's history. The browser's back button will go back to the previous spot on the page but so will the Back Button component. If you click the link three times you can select back (browser or component) three times and remain on the same page.

The behavior introduced in this PR is how CDS code was intended to work. In rcmp-release-prod branch on the Privacy Consent page the link behaves this way.

Perhaps we roll back this merge and discuss further. We should also discuss the idea of displaying a modal to inform user that their input will be lost when they select back (component or browser).

sdzhangtao commented 4 years ago

I am fine either way. We should let the user know that we changed the behavior. Don't think it's such a big deal since our forms are small, and most users never even bother to click the link anyway.