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.

Sign up for GitHub

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

Jump to bottom

Fix browser nav & remove the need to double-ENTER on the searchbar #298

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
+33 −26
Open

Fix browser nav & remove the need to double-ENTER on the searchbar #298

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3832725
make back nav more intuitive (every time you click search it adds to …
AbhiramTadepalli Oct 31, 2024
e9e6e10
Dynamically change page title based on searchTerms. Helps make back n…
AbhiramTadepalli Nov 1, 2024
57f4566
lint check
AbhiramTadepalli Nov 1, 2024
1ce794d
Trigger new search when clicking ENTER to select add an autocomplete …
AbhiramTadepalli Nov 2, 2024
c93e6a9
Fix formatting of the header (trailing comma, add 'Results - ' at the…
AbhiramTadepalli Nov 2, 2024
673bbf4
Remove state
TyHil Nov 8, 2024
11e2a2f
Bring the meta property for link preview into dashboard/index.tsx so …
AbhiramTadepalli Nov 8, 2024
9dc3e06
(revert later) add a test to diagnose link preview bug
AbhiramTadepalli Nov 8, 2024
17f3293
Revert "Remove state"
AbhiramTadepalli Nov 8, 2024
143f080
Reapply "Remove state"
AbhiramTadepalli Nov 8, 2024
a782977
Revert "(revert later) add a test to diagnose link preview bug"
AbhiramTadepalli Nov 8, 2024
96afc30
try to directly make the meta tag from the url
AbhiramTadepalli Nov 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 8 additions & 23 deletions src/components/common/SearchBar/searchBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ interface SearchProps {
*
* Styled for the splash page
*/
let wasEmpty = false; // tracks if the searchbar was empty before the new entry (to create a new browser navigation entry push())
const SearchBar = ({
manageQuery,
onSelect,
Expand Down Expand Up @@ -73,25 +72,13 @@ const SearchBar = ({
} else {
delete newQuery.searchTerms;
}
if (wasEmpty) {
// if the searchbar was cleared before this entry,
router.push(
{
query: router.query,
},
undefined,
{ shallow: true },
);
wasEmpty = false;
} //otherwise, just update the current navigation entry query
else
router.replace(
{
query: newQuery,
},
undefined,
{ shallow: true },
);
router.push(
{
query: router.query,
},
undefined,
{ shallow: true },
);
}
}

Expand Down Expand Up @@ -159,9 +146,6 @@ const SearchBar = ({

//update parent and queries
function onChange_internal(newValue: SearchQuery[]) {
if (newValue.length == 0) {
wasEmpty = true; // so that the next search creates a new navigation entry (push())
}
if (manageQuery === 'onChange') {
updateQueries(newValue);
}
Expand All @@ -181,6 +165,7 @@ const SearchBar = ({
if (newValue.length) setErrorTooltip(false); //close the tooltip if there is at least 1 valid search term
setValue(newValue);
onChange_internal(newValue);
Copy link
Member

Choose a reason for hiding this comment

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

Also, I think we can remove this line since the new line below covers its functionality

onSelect_internal(newValue); // clicking enter to select a autocomplete suggestion triggers a new search (it also 'Enters' for the searchbar)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TyHil Is this fine. newValue is a parameter for this function (it is an array with previous and the new SearchQueries). I am passing it in directly to onSelect_internal() instead of passing in value.

setValue sets value = newValue

I wanted to pass in value, but that does not get updated in time ig for onSelect_internal; newValue works fine and value eventually does get updated.

The other thing is there is a function handleKeyDown() that calls onSelect_internal when the enter key is pressed after adding all search terms (like I added cs1200 by clicking on it or SPACE or smtg, now I click enter).

Is it fine to have onSelect_internal to be called twice like this? Feels icky to mee

Copy link
Member

Choose a reason for hiding this comment

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

newValue is perfect

I suppose if we can remove handleKeyDown and the code still works then that would be good

TyHil marked this conversation as resolved.
Show resolved Hide resolved
}

//update parent and queries
Expand Down
1 change: 1 addition & 0 deletions src/pages/_app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ function MyApp({ Component, pageProps }: AppProps) {
<GoogleAnalytics gaId="G-CC86XR1562" />
<Head>
<title>UTD Trends</title>
<meta key="og:title" property="og:title" content="UTD Trends" />
<link
rel="icon"
type="image/png"
Expand Down
2 changes: 0 additions & 2 deletions src/pages/_document.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ function Document() {
name="theme-color"
content={tailwindConfig.theme.extend.colors.royal}
/>

<meta property="og:title" content="UTD Trends" />
<meta
property="og:description"
content="A data visualization tool built to help students view historical course and section data."
Expand Down
19 changes: 18 additions & 1 deletion src/pages/dashboard/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,17 +758,34 @@ export const Dashboard: NextPage = () => {
);
}

let pageTitle = '';
courses.map((term) => {
pageTitle += searchQueryLabel(term) + ', ';
});
professors.map((term) => {
pageTitle += searchQueryLabel(term) + ', ';
});
pageTitle =
pageTitle.lastIndexOf(', ') === pageTitle.length - 2
? pageTitle.substring(0, pageTitle.lastIndexOf(', ')) + ' - '
: pageTitle;

/* Final page */

return (
<>
<Head>
<title>Results - UTD Trends</title>
<title>{'Results - ' + pageTitle + 'UTD TRENDS'}</title>
<link
rel="canonical"
href="https://trends.utdnebula.com/dashboard"
key="canonical"
/>
<meta
key="og:title"
property="og:title"
content={'Results - ' + pageTitle + 'UTD TRENDS'}
/>
<meta
property="og:url"
content="https://trends.utdnebula.com/dashboard"
Expand Down
Loading