Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

closes #165: fix for navigation on 404 page #166

Closed
wants to merge 5 commits into from

Conversation

KirankumarAmbati
Copy link
Contributor

@KirankumarAmbati KirankumarAmbati commented Mar 3, 2019

Description of Changes:

Once a user lands on the 404 Page, he/ she is not able to migrate to any other page using any of the links. The UI is kind of freezed. On clicking any link, URL is getting updated but UI remains unchanged. My changes will address this issue.

@LaRuaNa
Copy link
Contributor

LaRuaNa commented Mar 3, 2019

There is a refactoring PR I'd wait for first. #157

@KirankumarAmbati
Copy link
Contributor Author

Sure. Please let me know once the PR #157 is merged. I'll pull those changes so that you can proceed with the approval and merging. Thanks!

@sagirk
Copy link
Contributor

sagirk commented Mar 3, 2019

/gcbrun

@antsmartian
Copy link

Thanks for the PR, but it would be great if you can provide your description short enough, so that people can know whats your fix is for and what change is expected (I know you had given in the heading, but link in the description would be great!). Thanks! @KirankumarAmbati

Co-Authored-By: KirankumarAmbati <kiran.chinna12520@gmail.com>
@KirankumarAmbati
Copy link
Contributor Author

Thank you @antsmartian for the suggestion. I updated the description. Please let me know if that looks good. Thanks!

Copy link
Contributor

@sagirk sagirk left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

One suggestion below, but no worries if you want to take care of it on a follow-up PR.

@sagirk
Copy link
Contributor

sagirk commented Mar 7, 2019

Please let me know once the PR #157 is merged. I'll pull those changes so that you can proceed with the approval and merging. Thanks!

@KirankumarAmbati #157 is finally merged. 🎉
Please rebase against master and resolve the merge conflicts. Let me know if you need any help.

@KirankumarAmbati
Copy link
Contributor Author

Thanks @sagirk for letting me know about #157

I took merges, resolved the merge conflicts and pushed the changes.

@sagirk
Copy link
Contributor

sagirk commented Mar 7, 2019

It shows no changes now. Can you check?

image

@sagirk
Copy link
Contributor

sagirk commented Mar 7, 2019

FYI, the navigation is currently no more included on the 404 page (it may be reinstated... follow the discussion).

@KirankumarAmbati
Copy link
Contributor Author

Ya. I just noticed that but I feel navigation is more user-friendly than a blank 404 page. May be we can get back to the navigation section once all things settle down.

@KirankumarAmbati
Copy link
Contributor Author

KirankumarAmbati commented Mar 7, 2019

@sagirk This PR is no more valid since the GraphQL part is removed from the 404 page. Closing it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants