Skip to content

Conversation

zoran995
Copy link
Collaborator

No description provided.

@zoran995 zoran995 requested review from na9da and micholm June 30, 2025 15:36

<h2>Internal server error</h2>
<p>Something went wrong on our server. Please send an email to <a href="mailto:data@pmc.gov.au">data@pmc.gov.au</a>, explaining what you were doing at the time it happened. This will help us track down the cause.</p>
<p><b>&larr; Return to <a href="/">NationalMap</a></b>.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this line be retained so the user has something to click on to get to somewhere that at least doesn't give an error page? I guess the destination should be made more generic though, perhaps just calling it "the map"?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that we want to leave the design and the content decisions about 400.html and 500.html pages up to a particular implementation, so it makes sense to keep them very minimal.

<body>
<h1 class="error-number">500</h1>
<div class="inline-block align-middle">
<h2 class="error-description" id="desc">Internal server error.</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think headings are typically without punctuation like periods/question marks/etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment for the 400.html page.

<p>You can perform these steps in any order as required to tune your display of spatial data.</p>


<h2><a class="anchor" name="feature-info">How to find out about a displayed feature</a></h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

Some parts of this file, like this subsection, seem generally useful for a novice user. Is the information already available elsewhere in the UI?

Copy link
Contributor

Choose a reason for hiding this comment

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

This can be done in the future if needed.
As far as I understand it, the present PR is about cleaning all up.

<body>
<h1 class="error-number">404</h1>
<div class="inline-block align-middle">
<h2 class="error-description" id="desc">Not found.</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about no period in heading and providing a clickable link to somewhere reasonable as for the 500 page.

Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment above about keeping it all minimal.
The punctuation here makes sense as it renders as in the picture below.
Screenshot 2025-07-30 at 1 43 40 PM

Copy link
Contributor

@niktayv niktayv left a comment

Choose a reason for hiding this comment

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

  • Tested locally, everything worked well.
  • Code changes LGTM.
  • Ran yarn prettier-check and got warnings for a couple of files (see below).
  • I suggest adding wwwroot/version.json to .prettierignore.
  • Running yarn prettier will fix lib/Styles/variables-overrides.scss (remove a space).
[warn] lib/Styles/variables-overrides.scss
[warn] wwwroot/version.json

Copy link
Contributor

@niktayv niktayv left a comment

Choose a reason for hiding this comment

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

Perhaps, we can ignore the prettier-check here.
Approved.

@zoran995 zoran995 merged commit 28f30a2 into main Aug 1, 2025
6 checks passed
@zoran995 zoran995 deleted the cleanup-nat-map branch August 1, 2025 11:15
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.

3 participants