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.

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

Nearby View: Add Location Field Search #1320

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

miles-grant-ibigroup
Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup commented Dec 9, 2024

Adds a location field search field to the nearby view. There are still some styling concerns, and the current location issue is annoying as well. Would be nice to get that fixed.

Should we have it try to render a string on first load? If we did that then the placeholder text would never appear and that's why I'm against it.

Should we add using this thing to the Percy tests?

Screenshot 2024-12-09 at 1 00 40 PM

Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup left a comment

Choose a reason for hiding this comment

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

This looks great!!! Found some opportunities for cleanup, also looks like there might be a few type errors

lib/components/viewers/nearby/nearby-view.tsx Show resolved Hide resolved
lib/components/viewers/nearby/nearby-view.tsx Outdated Show resolved Hide resolved
...nearbyViewCoords,
name: reversedPoint
}}
LocationIconComponent={() => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm this isn't showing up on chrome or safari
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, when it does show up it feels v close to the placeholder text? Probably better suited to a OTP-UI PR, but I guess just flagging it here

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah these might both be otp-ui issues!

lib/components/viewers/nearby/nearby-view.tsx Outdated Show resolved Hide resolved
@miles-grant-ibigroup
Copy link
Collaborator Author

These were some beautifully excellent catches thank you! All resolved

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

Successfully merging this pull request may close these issues.

2 participants