Add Dropdown Menu to the mobile IDE View #1513
Conversation
There was a problem hiding this comment.
I'm not seeing the same linting errors that you're seeing from PropTypes (I tried removing one one of the // eslint-disable-line statements and nothing new appeared. I noticed that this PR commits a package.json. Maybe it's installing a new React version/PropTypes version? Maybe you're using a different version of Node?
Also, on the desktop editor, the console heading got a little messed up:
Lastly, I'm noticing a few things commented out here and there which I would prefer is just removed. I've found that just leaving commented out code (which I am guilty of doing!) causes confusion.
|
Nice, I'll look into this! |
|
Should be good now. Console glitch was fixed, and eslint stopped complaining |
|
Quick note: I didn't delete the |
client/utils/custom-hooks.js
Outdated
| // Return values | ||
| const setRef = (r) => { ref.current = r; }; | ||
| const [visible, setVisible] = useState(true); | ||
| const trigger = () => setVisible(true); |
There was a problem hiding this comment.
I noticed that when you use Tab to get to the Dropdown button, and then hit Enter, the Dropdown will open, but if you hit Enter again, it won't close. Is it because trigger only sets visible to true when it should toggle it (i.e. visible = !visible)?
There was a problem hiding this comment.
You're right, thanks for catching that! I updated it and it now behaves exactly like the original dropdown lists ✨
|
Two things left I think:
|
| @@ -34,7 +34,8 @@ class App extends React.Component { | |||
| render() { | |||
| return ( | |||
| <div className="app"> | |||
| {this.state.isMounted && !window.devToolsExtension && getConfig('NODE_ENV') === 'development' && <DevTools />} | |||
| {/* FIXME: Remove false */} | |||
There was a problem hiding this comment.
I'm assuming you want to remove this for this PR!
|
This is working great! I think it is ready to merge. |
Part of the Mobile UI Project. This PR adds an
<OverlayManager />component, and a general-purpose<Dropdown />component. It creates a navigation dialog for the<MobileIDEView />.I have verified that this pull request:
npm run lint)Fixes #123