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

fix(search): navigation URL path change #506

Closed

Conversation

sourabpramanik
Copy link

Description

Changed the path name in navigating URL so now after selection of the search result from the list the user will be navigated to correct page.

And removing the filter in the dashboard won't lead the user to 404.

What type of PR is this? (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🎨 Style
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build
  • 🔁 CI
  • 📦 Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

This PR fixes both issues #505 and #2423 in open-sauced/app repo

Mobile & Desktop Screenshots/Recordings

fix

Steps to QA

1.go to https://hot.opensauced.pizza/
2. type rust
3. wait for the results, click on the first one to select it, then press Enter to navigate
4. try removing filter now

Added to documentation?

  • 📜 README.md
  • 📓 docs.opensauced.pizza
  • 🍕 dev.to/opensauced
  • 📕 storybook
  • 🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

NO

[optional] What gif best describes this PR or how it makes you feel?

Changed the path name in navigating route so now after selection of the search result from the list
the user will be navigated to correct page.

fix #2423
Copy link

netlify bot commented Jan 8, 2024

Deploy Preview for hot-sauced-ui ready!

Name Link
🔨 Latest commit c45773c
🔍 Latest deploy log https://app.netlify.com/sites/hot-sauced-ui/deploys/659c3441b9be0100081f1067
😎 Deploy Preview https://deploy-preview-506--hot-sauced-ui.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance:
Accessibility:
Best Practices:
SEO:
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks

Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.

Issue Reference

In order to be considered for merging, the pull request description must refer to a specific issue number. This is described in our Contributing Guide.
This check is looking for a phrase similar to: "Fixes #XYZ" or "Resolves #XYZ" where XYZ is the issue number that this PR is meant to address.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first Pull Request and thanks for taking the time to improve Open Sauced! ❤️! 🎉🍕
Say hello by joining the conversation in our Discord

Copy link
Member

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up!!!! 🚀

A few linting errors it looks like: please also address those! Thanks!!

src/components/Hero.tsx Outdated Show resolved Hide resolved
@sourabpramanik sourabpramanik requested a review from jpmcb January 8, 2024 17:45
@nickytonline
Copy link
Member

As soon as I type the letter r in the deploy preview, the screen goes blank and there are errors in the console.

CleanShot 2024-01-08 at 16 06 37

supabase.ts:120 {code: '42501', details: null, hint: null, message: 'permission denied for schema public'}
overrideMethod @ console.js:213
Ou @ supabase.ts:120
await in Ou (async)
(anonymous) @ Hero.tsx:50
(anonymous) @ useDidUpdate.js:28
Ki @ react-dom.production.min.js:244
ma @ react-dom.production.min.js:286
(anonymous) @ react-dom.production.min.js:282
z @ scheduler.production.min.js:13
B @ scheduler.production.min.js:14
Show 1 more frame
Show less
react-dom.production.min.js:189 TypeError: Cannot read properties of null (reading 'length')
    at bK (Hero.tsx:114:26)
    at Lf (react-dom.production.min.js:167:137)
    at Kl (react-dom.production.min.js:197:258)
    at im (react-dom.production.min.js:292:88)
    at am (react-dom.production.min.js:280:389)
    at s0 (react-dom.production.min.js:280:320)
    at ki (react-dom.production.min.js:280:180)
    at iu (react-dom.production.min.js:271:88)
    at tm (react-dom.production.min.js:268:429)
    at z (scheduler.production.min.js:13:203)

Hero.tsx:114 Uncaught TypeError: Cannot read properties of null (reading 'length')
    at bK (Hero.tsx:114:26)
    at Lf (react-dom.production.min.js:167:137)
    at Kl (react-dom.production.min.js:197:258)
    at im (react-dom.production.min.js:292:88)
    at am (react-dom.production.min.js:280:389)
    at s0 (react-dom.production.min.js:280:320)
    at ki (react-dom.production.min.js:280:180)
    at iu (react-dom.production.min.js:271:88)
    at tm (react-dom.production.min.js:268:429)
    at z (scheduler.production.min.js:13:203)

@sourabpramanik
Copy link
Author

Yes @nickytonline the supabase env variables VITE_SUPABASE_URL and VITE_SUPABASE_API_KEY are not working correctly.

I actually used the production supabase URL and API key ( from headers when you make requests ) to fix this issue 😁

@BekahHW BekahHW requested a review from bdougie January 22, 2024 15:38
@bdougie
Copy link
Member

bdougie commented Apr 1, 2024

Sorry for letting this one go stale @sourabpramanik. We have some updates that will make this PR not as useful.

open-sauced/app#2961
#516

I am updating the URL here, but let me know if you want to continue the path of improving the experience on this project.

@sourabpramanik
Copy link
Author

I am updating the URL here, but let me know if you want to continue the path of improving the experience on this project.

Sure thing @bdougie

@bdougie
Copy link
Member

bdougie commented Apr 16, 2024

This PR makes this not worth pursuing as is. We would need an entirely different approach for search.

Most like something similar to app.opensauced.pizza, where we use the GitHub API directly.

https://github.com/open-sauced/app/blob/beta/lib/hooks/useSearchRepos.ts

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