Skip to content

1757 mobx new routing#4679

Merged
soyarsauce merged 57 commits intonextfrom
1757-mobx-new-routing
Oct 30, 2020
Merged

1757 mobx new routing#4679
soyarsauce merged 57 commits intonextfrom
1757-mobx-new-routing

Conversation

@soyarsauce
Copy link
Copy Markdown
Contributor

@soyarsauce soyarsauce commented Aug 27, 2020

What this PR does

Addresses the first half (routing) of v7-maps-running-prerendering on v8
https://github.com/TerriaJS/drought-map/issues/195
(and the first half of porting #3494)

Additionally, I've disabled the IE11 tests given IE11 doesn't actually work for v8 at the moment.

What this PR doesn't do

As part of another issue/pr,

  • The styling for hitting the prerendered routes, that can come separately
  • Any extra checks for making UI elements that do route changes with anchor tagged elements instead of buttons

Checklist

  • n/a There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
  • I've updated CHANGES.md with what I changed.

soyarsauce and others added 30 commits May 29, 2019 18:34
Groups are also links now
explorerPanelIsVisible now ties into location routing
Visibility was not toggled correctly when
waiting for animation to finish
# Conflicts:
#	lib/ReactViewModels/ViewState.js
#	lib/ReactViews/DataCatalog/CatalogItem.jsx
#	lib/ReactViews/SidePanel/SidePanel.jsx
#	lib/ReactViews/StandardUserInterface/StandardUserInterface.jsx
#	package.json
#	test/ReactViews/StandardUserInterfaceSpec.jsx
# Conflicts:
#	doc/customizing/client-side-config.md
#	lib/ReactViewModels/ViewState.js
#	lib/ReactViews/DataCatalog/CatalogItem.jsx
#	lib/ReactViews/SidePanel/SidePanel.jsx
#	lib/ReactViews/StandardUserInterface/StandardUserInterface.jsx
#	package.json
#	test/ReactViews/StandardUserInterfaceSpec.jsx
@soyarsauce
Copy link
Copy Markdown
Contributor Author

@na9da as discussed, let me address the pr comments here when I get a moment, and you can start working on the other 2 points in the desc. Re: the second point, would just be checking that all the links that link you to certain routes are anchor elements - check the prod https://map.drought.gov.au/ for instances, the basic check is turn off javascript and see if you can traverse the catalog from /catalog/ onward

@na9da
Copy link
Copy Markdown
Collaborator

na9da commented Oct 27, 2020

An error pops up on start when loading a static URL. This is because Terria tries to load /proxyabledomains URL. #4834 might be relevant to this problem.

image

@soyarsauce
Copy link
Copy Markdown
Contributor Author

Re:

Would be great if we can get rid of the %2F%2F in the generated URLs, eg: http://localhost:3001/catalog/%2F%2FCesium%20Air

I'm not quite sure how to tackle this, we want to differentiate when a slash is for routing purposes, or if it's for ID purposes. (e.g. we need to be able to have confidence to always encode and decode a URL, without doing any double encoding/decoding etc).

Ideally we don't use such confusing characters in IDs, but is unfortunately a side effect of having IDs like //Cesium Air.

If we didn't encode it as such, we would end up with URLs like http://localhost:3001/catalog///Cesium%20Air and would prove difficult when trying to set up the routing. What do you think?
@na9da

@soyarsauce
Copy link
Copy Markdown
Contributor Author

soyarsauce commented Oct 27, 2020

An error pops up on start when loading a static URL. This is because Terria tries to load /proxyabledomains URL. #4834 might be relevant to this problem.

Great to see you've reproduced what I had, you'll have to investigate why this is an issue in v8 but not the v7 port (I'm thinking why is it not an issue in v7?*)

@na9da
Copy link
Copy Markdown
Collaborator

na9da commented Oct 27, 2020

For greater performance styled-components use CSSOM to add styling rules. This means when we pre-render the styles are absent from the dom. We can disable use of CSSOM when pre-rendering by passing a flag to the StyleSheetManager.

@na9da
Copy link
Copy Markdown
Collaborator

na9da commented Oct 28, 2020

Fixes for styling and proxying - TerriaJS/TerriaMap#496

@soyarsauce soyarsauce requested a review from na9da October 29, 2020 23:08
@soyarsauce
Copy link
Copy Markdown
Contributor Author

@na9da please check all good and we can do a release :) There is a minor problem at #4940 but we can address later

@na9da
Copy link
Copy Markdown
Collaborator

na9da commented Oct 29, 2020

Approving!
@soyarsauce - Let's tackle #4940 and #4941 later.

@soyarsauce soyarsauce merged commit 2957b74 into next Oct 30, 2020
@soyarsauce soyarsauce deleted the 1757-mobx-new-routing branch October 30, 2020 00:17
tephenavies added a commit that referenced this pull request Nov 2, 2020
soyarsauce added a commit that referenced this pull request Nov 4, 2020
…routing""

This reverts commit 093c61b.

# Conflicts:
#	lib/ReactViews/Preview/DataPreview.jsx
@soyarsauce soyarsauce mentioned this pull request Nov 4, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants