Add ctrl+click to the "View more info for this class" button, remove "back to search results" url. #238
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| <IconNotepad className="notepad-icon" /> | ||
| <div className="view-more-info-container"> | ||
| <IconNotepad className="notepad-icon" /> | ||
| <a |
There was a problem hiding this comment.
We should probably use instead of an tag, since it works with the React router and gives us this functionality still. I think with Next it also prerenders the pages that are being linked, so theres little to no waiting.
There was a problem hiding this comment.
Sorry, what should we use instead? If you’re referring to a Link tag, unfortunately that doesn’t allow ctrl clicking.
There was a problem hiding this comment.
oops yea I was talking about Link. Apparently someone had a similar issue (vercel/next.js#7218), and the solution was to just put the href in the Link tag. If that doesn't work I'd be ok with the plain a tag.
| loadClassPageInfo(); | ||
| }, [subject, classId]); | ||
| }, [subject, classId, router]); |
There was a problem hiding this comment.
Is router required here as a dependency?
There was a problem hiding this comment.
Yes, we get graphql errors otherwise.
Purpose
This ticket turns the "View more info for this class' button into a url, allowing it to be more easily copied and ctrl-clicked. In the process, I found a bug related to graphql making queries before all the react data is loaded; a conditional has been added to wait for the necessary data to load before any data is fetched.
I also decided to remove the "back to search results" url, since I felt that it was unnecessary - other search engines like YouTube and Google don't have back buttons on their sites, they just rely on the browser. I can definitely roll this back if you think it's better to have it!
Feel free to check out the vercel deployment and see this change for yourself!
Tickets
This ticket!
Contributors
@Lucas-Dunker
Pictures
Reviewers
Primary:
@pranavphadke1
Secondary:
@sebwittr @ananyaspatil