Ej/settings menu #732
Ej/settings menu #732
Conversation
|
Im not really sure what is going on but some of your changes are not showing up (eg. home.js). Maybe try closing and re-opening this PR |
|
I think you should be able to see the changes now @ErikSin ? |
|
Basically it's because git tracks commits and so despite reverting those commits they were technically already in master. Solution was revert the reverts 🙃 |
|
The setting tab should not have the red bar, and the line on the right should go all the way to the bottom (not just the existing menu items) |
There was a problem hiding this comment.
We are using JSdoc to leverage typescript typesafety. Before we can merge this, we need to refactor the code to use that please!
| ] | ||
|
|
||
| export const SettingsView = () => { | ||
| const [menuItem, setMenuItem] = useState(null) |
There was a problem hiding this comment.
Define this useState so it is not type any
The easiest way to do this in JSDOC is to set an initial state as a variable using jsdoc:
/** @type {tabs['tabId'] | null} */
const initialMenuState = /** {const} */ (null)
const [menuItem, setMenuItem] = useState(initialMenuState)You will need to define tabs with @typedef.
|
aaah I hadn't seen but the merge has wrecked the code, gotta love git sometimes 🥲 |
|
Can you extend the menu content to be full screen? |
| aboutMapeoSubtitle: 'Version and build number' | ||
| }) | ||
|
|
||
| const tabs = /** @typedef {const} */ [ |
There was a problem hiding this comment.
And then we want to define this as
/** @type {import('./SettingsMenu')tab[]} */
This means tabId can only equal the tabs that we set, instead of a generic string
|
I think this is ready to go, just before I merge just wanted to confirm that this is merging into master.. is that correct? @ErikSin |
First time contributor checklist:
Contributor checklist:
Script and updated it if necessary.
npm install --package-lockDescription
closes #728
Adds a Settings menu to main layout tabs and an About Mapeo submenu.
Note: currently contains a test item on the main settings menu to indicate layout when more than one item present. Will remove before PR gets merged.
Preview