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
[docs][Autocomplete] Fix GoogleMaps demo #35545
Conversation
|
b80b128
to
fce90bb
Compare
fce90bb
to
09842f8
Compare
61ffec2
to
2f15e33
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.
Thanks for looking at this bug, there are quite a few open-source projects just to solve this problem, so people will appreciate a quality demo 👍. A few more ideas to improve the Google Maps place demo:
- There is an overflow issue, e.g. when searching for "zzzz"
- Should we customize the
noOptionsText
prop? Maybe like "No places" would make more sense than "No options"? - The warning "Before you can start using the Google Maps JavaScript API and Places API, you must sign up and create a billing account." feels unclear. It doesn't explain why. I think that it should mention that it's to get an API key.
- We frequently hit a rate limit error. I have configured some of it in our Google Cloud dashboard so we stay inside the free tier, but I don't understand why we get them. Maybe we could set the throttle at 400ms rather than 200ms?
- Considering how we solve the problem of out-of-order requests, using throttle makes no sense. We effectively have a debounce, so we might as well use a debounce to make fewer network requests.
Fixed.
Agree. I will go with
Yes, "Before you can start using the Google Maps JavaScript API and Places API, you need to get your own API key." sounds better. |
Replaced |
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 have pushed a few more changes that were not directly related to the initial problem but seemed to be a good opportunity to modernize the example, to use the features that were not available when the demo was first created.
I can't find anything else to improve 👍 , this is a much better demo 😄
Off-topic. I have just realized that the undo Ctrl+Z and redo shortcuts do nothing on our autocomplete. It's a bit frustrating. It works in Gmail: I can't find any UI component that supports this (select2, react-select, downshift, react-spectrum, antd, they don't), so maybe an opportunity to get an edge on the UX side. Or maybe it's overkill 🤷♂️ |
I don't really think it's an overkill ;) It's quite important for UX that undo/redo works. |
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.
Closes #35391
Problem:
Before:
(1) Go to https://mui.com/material-ui/react-autocomplete/#google-maps-place
(2) Enter "Darse de Baudour"
(3) Demo crashes
After:
(1) Go to https://deploy-preview-35545--material-ui.netlify.app/material-ui/react-autocomplete/#google-maps-place
(2) Enter "Darse de Baudour"
(3) Demo doesn't crash