General tidy up on Find #103

samuelhwilliams · 2024-05-02T08:15:13Z

Change description

This one ran away with me a bit. It started as me just wanting to fix a weird margin issue on the login page, but then I kept finding other bits of oddness.

Review commit by commit for more info.

This has ended up as a bunch of just general tidying up, really.

Unit tests and other appropriate tests added or updated
README and other documentation has been updated / added (if needed)
Commit messages are meaningful and follow good commit message guidelines (e.g. "FS-XXXX: Add margin to nav items preventing overlapping of logo")

Screenshots of UI changes (if applicable)

	Before	After
Login page: white margin around entire body, no phase banner
download page, too much space above header, redundant back link

We don't display any cookie banner on the find frontend currently, and this JS expects some specific cookie-related elements to be in the HTML, so we just get null element errors in the JS console. Remove them until we address cookies properly at some future time.

We weren't extending the base template for the login page, leaving us with some invalid HTML, causing bad styling. We also weren't showing the phase banner for some reason. But we should be.

We were adding our own `main` elements on these pages, but the base template provides this for us, so we were just duplicating them. This removes the duplication. This was also causing excessive space above the h1 elements, which is now fixed.

`download` gets assigned to the `download` button of the form, so we shouldn't reuse it for the accordion. The id of the accordion is broadly irrelevant so we can just prefix it with `accordion-` to disambiguate.

Possibly due to inconsistent indenting, there was a missing closing `div` tag, which could cause HTML rendering issues in certain cases. In one specific case, on the download page, selectItems was the last part of an accordion, and it caused the styling for the end of the accordion to leak beyond the intended range. The accordion inserts a bottom border line after the each accordion section, and this leak was causing that border line to appear well beyond where we expected (in this case, after the download button and 'Access the funding glossary' link. This only happened after fixing some other HTML issues on the page.

And add some tests around the back link Some probably-unused pages had duplicate back links, so let's remove those too... (/cookies, /accessibility, /privacy)

The start page isn't part of the main user journey any more. As soon as a user logs in or access the root page (`/`), they are redirected to `/download`. It looks like a decision was made to skip the start page as being fairly unnecessary, but the code/view wasn't ever removed. So let's tidy it up now. Just in case any users have it bookmarked, we add a redirect so that things don't break for them.

The base template provides the width wrapper already. Doubling up on this adds extra spacing above the header which looks a bit weird. Let's remove it - as well as the redundant beforeContent block.

We have three pages that were bootstrapped into this app from the start: /cookies, /privacy, and /accessibility /cookies does not accurately reflect the cookies that we actually store and use on Find. Having an inaccurate cookie policy page is probably worse than not having one at all, so let's remove it until we can do the work to make it work and make it accurate. /privacy has no content on it. We can add this URL back as and when we actually do the work to describe our position around privacy accurately. /accessibility just contains the placeholder accessibility statement from GOV.UK with all of the placeholders still in line. So it looks like we haven't ever had an accessibility statement written up. It probably looks worse for us to be serving an unedited accessibility statement than none at all. For all of these views, we now just log an error on access, so that if users do actually try to view these things, we'll get some warning in Sentry, which may help prioritise it (but regardless of if users try to look at the page, we still need to do the work)

samuelhwilliams added 11 commits May 1, 2024 22:34

Fix HTML/styling on login page

850ab3b

We weren't extending the base template for the login page, leaving us with some invalid HTML, causing bad styling. We also weren't showing the phase banner for some reason. But we should be.

Remove unused div with meaningless class

105c8f6

fix: add a space

26af317

fix: remove duplicate element ID

87ac20e

`download` gets assigned to the `download` button of the form, so we shouldn't reuse it for the accordion. The id of the accordion is broadly irrelevant so we can just prefix it with `accordion-` to disambiguate.

fix: hide back link on download page

c31167d

And add some tests around the back link Some probably-unused pages had duplicate back links, so let's remove those too... (/cookies, /accessibility, /privacy)

fix: remove duplicate styling from help page

3b88a9f

The base template provides the width wrapper already. Doubling up on this adds extra spacing above the header which looks a bit weird. Let's remove it - as well as the redundant beforeContent block.