Skip to content
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.

Sign up for GitHub

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

Jump to bottom

improve split panel #852

Closed
hamed-musallam opened this issue Jan 7, 2025 · 3 comments · Fixed by #862
Closed

improve split panel #852

hamed-musallam opened this issue Jan 7, 2025 · 3 comments · Fixed by #862
Assignees
@stropitek

Comments

@hamed-musallam
Copy link
Contributor

hamed-musallam commented Jan 7, 2025 •
edited
Loading

  1. Add an interactionKind Property

  • Current issue:
    When a user interacts with the splitter (e.g., clicks the "..." button), the closed property no longer controls the panel's open/close state.

  • Proposed solution:
    Introduce an optional property called interactionKind, which can take one of two values:
    1- forced: Ensures the panel behavior is fully controlled programmatically.
    2- undefined: Allows default behavior without restrictions.

  1. Refactor the closed property:

  • Current issue:
    The closed property is overloaded with two types of values:
    Boolean: Directly controls whether the panel is open or closed.
    String: Represents a size threshold for automatic closing.

  • Proposed solution:
    The closed property should only accept a boolean value for controlling the open/close state.
    Introduce a new property closeThreshold to handle automatic closure based on panel size.

  1. Add an onClose Callback:

  • Current issue:
    When using the size threshold for automatic closing, there’s no way to track whether the panel is currently closed.

  • Proposed solution:
    Add an onClose callback function that triggers whenever the panel is closed, regardless of whether it was closed manually or automatically.

  1. Improve automatic close behavior (I'm not sure what is the best way):

  • The automatic closing feature should always be active, based on the closeThreshold. I think we need to differentiate between two scenarios:
    1- User Interaction: When the user changes the panel size using the "..." button.
    2- Container Resizing: If the container size causes the panel to shrink below the threshold, it should automatically close.

  • feat: implement right-side panels bar cheminfo/nmrium#3318
@stropitek
Copy link
Contributor

  1. Add an interactionKind Property
  • Current issue:
    When a user interacts with the splitter (e.g., clicks the "..." button), the closed property no longer controls the panel's open/close state.
  • Proposed solution:
    Introduce an optional property called interactionKind, which can take one of two values:
    1- forced: Ensures the panel behavior is fully controlled programmatically.
    2- undefined: Allows default behavior without restrictions.

I prefer that we use this typical react pattern:

  • If the property is undefined, it is uncontrolled (which means automatically controlled by the component)
  • If the property is defined, it is controlled. We must introduce a callback prop (something like onClosedChange) which is called when we click the separator or when the closed value changes for any other reason (like the example with the closeThreshold).
  • The component is not supposed to switch from being uncontrolled to controlled and vice versa (print a warning if that happens).

2. Refactor the closed property:

SGTM

3. Add an onClose Callback:

SGTM, as also proposed above. Would use onClosedChange, the change suffix being typically used in this controlled / uncontrolled pattern.

4. Improve automatic close behavior (I'm not sure what is the best way):

Maybe explain exactly what is not working in the current implementation and create a separate issue for that.

@stropitek
Copy link
Contributor

@hamed-musallam Do you plan to work on this?

Seemed like a good idea in the first place.

@stropitek stropitek self-assigned this Feb 19, 2025
@stropitek stropitek linked a pull request Feb 19, 2025 that will close this issue
feat: improve split panel #862
Merged
@stropitek stropitek mentioned this issue Feb 19, 2025
Merged
stropitek added a commit that referenced this issue Feb 20, 2025
@stropitek
feat(SplitPane): rework SplitPane API
4cf816c 
Refactor the SplitPane API to be more consistent and idiomatic.
Make the `open` and `size` state of the component controllable.

BREAKING CHANGE: The `onToggle` prop has been renamed to `onOpenChange`
and receives true when the split pane opens (previously received false). 
Contrary to the previous onToggle, onOpenChange is called in any
circumstance when the split pane state changes, for example also when it
automatically closes after it reaches the `closeThreshold` size.
The original behavior of the `closed` and `size` props has changed.
Their behavior were a mix of a control value prop and default value prop
for internally managed state.
There is now a clear separation between control props (`open` and `size`)
and default value props `defaultOpen` and `defaultSize`.
For example for the open state you should either:
- Use `open` to control if the SplitPane is open or not, and `onOpenChange` to react to changes. Using `defaultOpen` will have no effect.
- Use `defaultOpen` to set the initial value (only affects first render), with state changes being handled internally by the component.
If you used `closed` with a number, use the new `closeThreshold` prop instead.


Closes: #852
@hamed-musallam
Copy link
Contributor Author

@stropitek

Great job !!!! 🎉 Thank you! Sorry, I missed your message, I had planned to work on this, but the past couple of weeks weren’t great, and then… well, the issue I opened completely vanished from my brain! 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@stropitek stropitek

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: improve split panel zakodium-oss/react-science
2 participants
@stropitek @hamed-musallam