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

Place redirect smoothing #4732

Merged
merged 4 commits into from
Nov 15, 2024

Conversation

nick-next
Copy link
Contributor

Description

This PR updates aspects of the flow, layout and styling of the redirect process for when a the search results give a "place overview" and the svSource is "unfulfilled", with the goal of removing redirect flashes and FOUCs.

The following changes were made to make the process visually smoother:

  • In the event that the a page is poised to redirect (i.e., the above criteria is true), the loading state is maintained. This prevents the intermediate "Place Overview" page from displaying just before the redirect, removing the first flash.
  • In the place page that follows (for example, place/country/CAN), the layout was adjusted so that a FOUC was removed while the content was being loaded in.
  • The page overview now fades in when it loads, softening the appearance of the load.
  • A small spinner was added onto this page to give more of a loading sense.

@nick-next nick-next requested review from dwnoble and beets November 13, 2024 03:19
@dwnoble dwnoble requested a review from gmechali November 13, 2024 23:51
@@ -69,7 +69,7 @@ <h3 id="locale" data-lc="{{ locale }}"></h3>
<div id="place-summary" class="row-col place-summary-container">{{ place_summary }}</div>
<div id="main-pane" class="row"></div>
{# TRANSLATORS: A message shown on the page while the content is loading. #}
<div id="page-loading" className="mt-4">{% trans %}Loading...{% endtrans %}</div>
<div id="page-loading" className="mt-4">{% trans %}<svg xmlns="http://www.w3.org/2000/svg" height="24px" viewBox="0 -960 960 960" width="24px" fill="#5f6368"><path d="M480-80q-82 0-155-31.5t-127.5-86Q143-252 111.5-325T80-480q0-83 31.5-155.5t86-127Q252-817 325-848.5T480-880q17 0 28.5 11.5T520-840q0 17-11.5 28.5T480-800q-133 0-226.5 93.5T160-480q0 133 93.5 226.5T480-160q133 0 226.5-93.5T800-480q0-17 11.5-28.5T840-520q17 0 28.5 11.5T880-480q0 82-31.5 155t-86 127.5q-54.5 54.5-127 86T480-80Z"/></svg><p>Loading</p>{% endtrans %}</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wrap the {% trans %} tag around the actual text, in this case the word "Loading"

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add comment describing the SVG and give a source reference where it came from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated and ready for re-review!

For the SVG: we've used an SVG rather than the font as part of a move away from using the font-based Material icons, as the font-based icons are prone to content flashes while the font loads.

One of the next PRs coming will replace the inline SVGs (including this one) as well as remaining font-based material icons with an organized set of icon components.

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds good to me!

@nick-next nick-next force-pushed the place-redirect-fixes branch from 462ba7f to 65bcba9 Compare November 14, 2024 03:44
@nick-next nick-next requested a review from dwnoble November 14, 2024 03:48
Copy link
Contributor

@dwnoble dwnoble left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Contributor

@gmechali gmechali left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for getting this working!

pablonoel and others added 4 commits November 14, 2024 17:15
…ntent immediately before the redirect invokes).
…on is only called once, and it is only called in a context where we are sure that the pageMetaData exists.
@nick-next nick-next force-pushed the place-redirect-fixes branch from 65bcba9 to c1dae81 Compare November 15, 2024 01:43
@nick-next nick-next merged commit c7dc534 into datacommonsorg:master Nov 15, 2024
8 of 9 checks passed
@nick-next nick-next deleted the place-redirect-fixes branch November 15, 2024 02:07
gmechali pushed a commit to gmechali/website that referenced this pull request Nov 21, 2024
## Description

This PR updates aspects of the flow, layout and styling of the redirect
process for when a the search results give a "place overview" and the
svSource is "unfulfilled", with the goal of removing redirect flashes
and FOUCs.

The following changes were made to make the process visually smoother:
* In the event that the a page is poised to redirect (i.e., the above
criteria is true), the loading state is maintained. This prevents the
intermediate "Place Overview" page from displaying just before the
redirect, removing the first flash.
* In the place page that follows (for example, `place/country/CAN`), the
layout was adjusted so that a FOUC was removed while the content was
being loaded in.
* The page overview now fades in when it loads, softening the appearance
of the load.
* A small spinner was added onto this page to give more of a loading
sense.

---------

Co-authored-by: Pablo Noel <[email protected]>
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.

4 participants