adds a collapsible "create new app" section #163
Conversation
|
should run now pretty. |
src/containers/apps/Apps.tsx
Outdated
| hasPersistency | ||
| ) | ||
| <details id="create-new-app"> | ||
| <summary style={{display: 'none'}}></summary> |
There was a problem hiding this comment.
Look into hide-on-deman usage in the code.
There was a problem hiding this comment.
what is the meaning of the "-on-demand" part?
Note: i hide the summary element, as the button to open the details element needs to be somewhere else.
There was a problem hiding this comment.
Yes, you can just create a div element and use hide-on-demand class to hide it and show it when it needs to. There are many examples of that in the code.
src/containers/apps/AppsTable.tsx
Outdated
| }} | ||
| > | ||
| <Button | ||
| onClick={() => document.getElementById('create-new-app')?.toggleAttribute('open')} |
There was a problem hiding this comment.
This is an antipattern in react. You must use callbacks and redux if needed.
There was a problem hiding this comment.
i tried to implement the minimalistic change here and relied on basic html5 feature. Since there is no state-tracking possibility of the "open" state of the element, the only other approach would be to use useRef.
Since Apps.tsx and AppsTable.tsx are separated i don't have the exact overview of your hierarchy, so i found that this is the simplest approach. Also, it is just frontend only.
But feel free to adjust it if you are not happy with my approach here.
There was a problem hiding this comment.
Typically, you should just use a call back if it's the parent, or use redux. Modifying DOM elements are not recommended approach when using react.js.
If you want you can leave the pr as is. I'll make the change later.
| <Button | ||
| onClick={() => document.getElementById('create-new-app')?.toggleAttribute('open')} | ||
| type="primary" | ||
| > |
There was a problem hiding this comment.
The button needs to hide when the create app panel is visible.
There was a problem hiding this comment.
Also create app panel needs to have a close button that result in this button being visible again.
fixes caprover/caprover#2171
This is just a small ui adjustment and should not break anything. It uses standard html
<dialog>functionality.Screenshots of this change is in the linked issue