-
Notifications
You must be signed in to change notification settings - Fork 5
Core 1060 port separatemap page to ts #2766
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
Conversation
| } | ||
|
|
||
| function showTooltip(schoolInfo, flyThere) { | ||
| function showTooltip(schoolInfo: AugmentedInfo) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing was using flyThere
56453ee to
10e14a0
Compare
| const mapZoom = isMobileDisplay() ? 2 : 3; | ||
| const map = React.useMemo( | ||
| () => new Map({ | ||
| () => createMap({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes. It wasn't a constructor.
| (event) => setInstitution(event.target.value), | ||
| [setInstitution] | ||
| ); | ||
| const options = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having this defined inside the function was causing rerenders. This file is shown as changes in the first commit.
|
|
||
| .search-clear { | ||
| cursor: pointer; | ||
| appearance: none; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it from a clickable icon with role="button" to a button element.
| function SearchAndClear({ | ||
| placeholder='Search by country, state, city, or institution name', | ||
| textValue='', | ||
| placeholder, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only called by one function, and that provides values for these parameters.
has Update result-box.tsx
10e14a0 to
28b9816
Compare
| @@ -0,0 +1,152 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes in the first commit
| @@ -0,0 +1,61 @@ | |||
| import {useState, useEffect} from 'react'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See first commit
| resultsOrSchool?.map((school) => | ||
| <ResultBox | ||
| model={school} totalCount={resultsOrSchool.length} key={school.pk} | ||
| model={school} key={school.pk} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently the totalCount parameter went away some time ago in ResultBox.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test was subsumed by separatemap.test
28b9816 to
8e617a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test subsumed by new tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File diffs are in the first commit
CORE-1129
File changes are in the first commit. Tests are in the second commit. Prettier is the third.
I have annotated some unexpected changes. For files that show as deleted and created, you can see them as diffs in the first commit.