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

Adding Loading Page #1857

Merged
merged 3 commits into from
May 22, 2024
Merged

Adding Loading Page #1857

merged 3 commits into from
May 22, 2024

Conversation

KennethSkylight
Copy link
Collaborator

PULL REQUEST

Summary

Adding the ECR Loading Skeleton. How it fits in with the rest of the application will happen when the loading.js file can handle loading

Related Issue

Fixes #1760

Checklist

image

@sarahtress
Copy link
Collaborator

@KennethSkylight is it possible to download the whole screen and send here? just wanna get a sense of what the full page looks like! what you sent is looking great though!

@angelathe
Copy link
Collaborator

Code looks good, but @BobanL are the nextJS improvements relevant here? Or can the loading UI stuff only be fixed after your refactoring changes are merged?
@KennethSkylight For reference, https://nextjs.org/docs/app/building-your-application/routing/loading-ui-and-streaming

@BobanL
Copy link
Collaborator

BobanL commented May 21, 2024

Code looks good, but @BobanL are the nextJS improvements relevant here? Or can the loading UI stuff only be fixed after your refactoring changes are merged? @KennethSkylight For reference, https://nextjs.org/docs/app/building-your-application/routing/loading-ui-and-streaming

Yep these changes currently rely on the improvements I'm making, so right now these will not be visible until my changes get done.

@angelathe
Copy link
Collaborator

Code looks good, but @BobanL are the nextJS improvements relevant here? Or can the loading UI stuff only be fixed after your refactoring changes are merged? @KennethSkylight For reference, https://nextjs.org/docs/app/building-your-application/routing/loading-ui-and-streaming

Yep these changes currently rely on the improvements I'm making, so right now these will not be visible until my changes get done.

Ok thanks! @KennethSkylight could you write a ticket to follow up later on making the loading UI files match the new NextJS structure after @BobanL gets his PR in?

@KennethSkylight KennethSkylight added this pull request to the merge queue May 22, 2024
Merged via the queue into main with commit 3a59aac May 22, 2024
45 checks passed
@KennethSkylight KennethSkylight deleted the kenneth/loading-page-2 branch May 22, 2024 14:41
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.

Create loading skeleton page
4 participants