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.

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

onPanelWidthChange for EuiResizeableContainer only provides sizes for two panels #3466

Closed
atifsyedali opened this issue May 12, 2020 · 8 comments

Comments

@atifsyedali
Copy link

First, thank you for this amazing library of components!

Problem:

  1. When I attach an onPanelWidthChange on an EuiResizableContainer, the provided argument only gives sizes for two panels that are currently change.

  2. Furthermore, it assumes those two panels are the only ones in the container, such that one panel takes up the space of the other panels not indexed in the provided object.

  3. I have to end up doing _.extend({}, oldState, newState) but this is problematic too because of (2) above.

Not sure how to solve this atm. Any help would be greatly appreciated.

@chandlerprall
Copy link
Contributor

@thompsongl I think it makes sense to fire the onPanelWidthChange callback with all of the sizes, instead of just the two affected, thoughts?

I could not replicate point 2. For testing, I modified the docs' Horizontal resizing with three panels section to log the sizes object, and those values looked correct (not consuming the entire space).

src-docs/src/views/resizable_container/resizable_container_three_panels.js

import React, { useCallback, useState } from 'react';

import { EuiText, EuiResizableContainer } from '../../../../src/components';

const text = require('!!raw-loader!./lorem.txt');

export default () => {
  const [sizes, setSizes] = useState({ left: 25, middle: 25, right: 50 });
  const onPanelWidthChange = useCallback(newSizes => {
    setSizes(oldSizes => ({
      ...oldSizes,
      ...newSizes,
    }));
  });
  return (
    <EuiResizableContainer
      style={{ height: '400px' }}
      onPanelWidthChange={onPanelWidthChange}>
      {(EuiResizablePanel, EuiResizableButton) => (
        <>
          <EuiResizablePanel id="left" size={sizes.left} initialSize={100 / 3} minSize="50px">
            <EuiText>
              <p>{text}</p>
            </EuiText>
          </EuiResizablePanel>

          <EuiResizableButton size="l"/>

          <EuiResizablePanel id="middle" size={sizes.middle} initialSize={100 / 3}>
            <EuiText>
              <p>{text}</p>
            </EuiText>
          </EuiResizablePanel>

          <EuiResizableButton size="l"/>

          <EuiResizablePanel id="right" size={sizes.right} initialSize={100 / 3} minSize="10%">
            <EuiText>
              <p>{text}</p>
            </EuiText>
          </EuiResizablePanel>
        </>
      )}
    </EuiResizableContainer>
  );
};

@atifsyedali
Copy link
Author

@chandlerprall You are right about (2). Apologies -- my math was off. My third column was tiny so I rounded up in my head.

On a related note, is there a way to not make EuiResizeableContainer a controlled element when onPanelWidthChange is supplied? For example, to only receive the callback onPanelWidthChange so that the values can be persisted, and the size is set with initialSize instead.

Because right now, as soon as you specify the onPanelWidthChange attribute, the container becomes controlled and you have to use the size property, which means you have to go through setting state, which means rerendering the component that is rendering the resizable container, which means doing some calculations to figure out the columns again (at least for my app).

@thompsongl
Copy link
Contributor

Thanks for the feedback, @atifsyedali

Just to clarify, it sounds like your ideal case is:

  1. Have an uncontrolled EuiResizeableContainer (no size prop in use)
  2. Subscribe to panel size changes for persistence
  3. Use those persisted panel size values to set initialSize for initialization/reload

@atifsyedali
Copy link
Author

@thompsongl yes that would be ideal!

@shrey
Copy link
Contributor

shrey commented Jun 16, 2020

I'd like to work on this one

@shrey
Copy link
Contributor

shrey commented Jun 17, 2020

@thompsongl So, for the persistence of the size, what do you think should be used, redux-persist perhaps?

@thompsongl
Copy link
Contributor

@shrey In this case, the data persistence in question would be done entirely in the consuming application; EUI would have no part in how those size values are stored. Redux is certainly an available option for apps using EUI, but we make no prescription.

The necessary changes for EUI would be:

  1. Change the onPanelWidthChange callback prop to be called with the widths of all panels in the EuiResizeableContainer.
  2. Allow the onPanelWidthChange prop to be set and called without the size prop also being set.

@thompsongl
Copy link
Contributor

Resolved by #3630

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants