Remove route.resolve feature
#4607
Conversation
There was a problem hiding this comment.
Code looks good to me, regarding the LoadingState I do feel we should have something better (not in this scope, but we should address it in case).
As for other pages, the dashboard currently doesn't have a LoadingState (it's just blank) and the Alert Page has this spinner, but with a margin-top: 30px. If the matter is a quick win I'd go with just adding this margin.
I also noticed that different pages have different spinners. I was planning to revisit all that stuff together (it will also affect ItemsList component), and for me it feels a bit out of scope of this PR. |
| function QueryPageWrapper({ queryId, onError, ...props }) { | ||
| const [query, setQuery] = useState(null); | ||
| const onErrorRef = useRef(); | ||
| onErrorRef.current = onError; |
There was a problem hiding this comment.
Why do we need to persist the onError callback?
What type of PR is this? (check all applicable)
Description
During React migration of Router we decided to temporary keep
resolvefeature like inangular-router(this feature allows route to define dependencies that should be resolved before rendering page). Till now only Query View and Editor pages were using this feature, so it's time to remove it.Now query pages will load query object themselves showing spinner while loading. It's a little bit different from previous behavior (with
resolvefeature previous page stays visible until new is completely ready), but considering that other pages already work in the same manner - it shouldn't be an issue.Related Tickets & Documents
Fixes #4605