compactMode: add collapsible functionality + reog job card layout #697
Conversation
|
hey @felixmosh, modified this and should be ready to merge now |
|
Cool, I'll review it asap |
|
I've reviewed the PR, it looks buggy... "Collapse job" should define if the card can be collapseable or not (I prefere the full card view), this means that it should add / remove the arrow button. |
| max-height: 500px; | ||
| } | ||
|
|
||
| .card + .card { | ||
| margin-top: 2rem; | ||
| margin-top: 0.75rem; |
| @@ -3,14 +3,13 @@ | |||
| box-shadow: 0 1px 1px 0 rgba(60, 75, 100, 0.14), 0 2px 1px -1px rgba(60, 75, 100, 0.12), | |||
| 0 1px 3px 0 rgba(60, 75, 100, 0.2); | |||
| border-radius: 0.25rem; | |||
| padding: 1em; | |||
| padding: 0.66em; | |||
| React.useEffect(() => { | ||
| if (isOpen === undefined) | ||
| setIsOpen(!jobUrl ? true : !collapseJob) | ||
| }), []; |
There was a problem hiding this comment.
it definitelly works, try it :)
this is "onComponentMount" so the initial state is undefined:
- If we are on a job page, the card is instantly expanded, and then theres no collapse button.
- If we are on the list page, the card takes the value defined in the app setting and from them controlled by only the state
There was a problem hiding this comment.
It doesn't, since the array of deps is outside of the useEffect :P
There was a problem hiding this comment.
I reworked the logic to make it "useEffect-free" and handle the global state "live":
- It defaults to the state defined globally
- if the state at job level is changed, this is this is preserved even if the global state changed after, because now the job uses the local as its source of truth.
globalCollapse.mov
packages/ui/src/components/JobCard/Timeline/Timeline.module.css
Outdated
Show resolved
Hide resolved
This is not needed and it adds unneccesary complexity to the state, a user will normally just define this for the whole session/forever, wont toggle this all the time, but yeah I could make make it work as desired.
In this case a user would be in need of clicking like crazy for collapsing cards all the time to have a better view of the jobs that are being consumed by the second (if jobs are being properly named you most probably can infer the data they have inside an only would need to expand for specific ones which errors or logs). This would be completely unusable tbh. |
|
@ivnnv think about it, I want the current view (without the clutter of collase expand at all... maybe only the change of the extraction of the id location to the head). When I enable the |
No yeah, I get this point, its def doable and expected if all of the other settings work real-time 👍🏽 |
|
@felixmosh this should be ready as per your requested changes now |
|
Thank you, released in |
Yeah this definitelly had a regression bug when I introduced getting the collapseJob settingsStore, this also corrects the double button, having the
Collapsible.Triggeris not really needed if we control the state in our Button.