leefsmp / Re-Flex

Resizable Flex layout container components for advanced React web applications
https://leefsmp.github.io/Re-Flex/index.html
MIT License
638 stars 73 forks source link

Is there any way to access all of ReflexElements' sizes at once? #142

Open growthwp opened 3 years ago

growthwp commented 3 years ago

Currently, the only way to get flex values are to listen to the onResizeStop but that only gives access to the data of the two ReflexElement being resized. I checked the docs, the relevant piece seems to be: https://gist.github.com/leefsmp/28ceb226d119d179d1e94466d2dd1a8b#file-re-flex-demo-storage-jsx

However, this misses a few cases:

1) What do we do if we have more than 2 items? The resize event will only fire for two items but ignore the third (as it should). We're left to guess the flex of the 3rd, 4th,... item.

2) Tied to 1 but data can only be retrieved when a resize happens. While you can provide initial flexes, let's assume from local storage, you'd need to have performed an onResizeStop event on all the ReflexElement components for you to be able to store ALL the values.

This can be mitigated by providing initial flexes on all the elements and then updating these that need updating on the onResizeStop event, but was wondering if this information was exposed somewhere.

leefsmp commented 3 years ago

I've never had the need to retrieve flex values in that way, so yeah there is actually no way but it should be doable to add all elements as part of the resize event. Could you give more details about the use case for accessing all flex values?

As you mentioned, a workaround can be to set or guess all default flex values from calling code then set resize event on each element and keep track of the different flexes.

growthwp commented 3 years ago

Well, in our case, this is vital. I'm not sure what the library is used for at length (so, this could be low-prio) but we have N columns (each column is a ReflexElement), now, wouldn't it be nice to:

Basically, I'm just trying to pass down an object which holds all of the flex values to each ReflexElement so that I can do some things.

It can be done as of now and I agree this is a matter of opinion but, provided this is easy to implement, it's way more elegant.

Thanks for the library (and the continuous support), the cross-browser compat and it being able to do so much is God-sent. All I'm proposing is what to me seem clear improvements. Please know that I'm not usign the word "clear" in a cocky way but I've already written the logic for these (if you recall the "what do you do when you have a dynamic number of elements?") and while it's not contrived - people can tell what's going on, it's so much more effort than it needs to be.

leefsmp commented 3 years ago

Sorry for the delay, quite busy these days... It should be straightforward to expose an onResize prop on ReflexContainer that contains an array with all the elements and their respective flex will be accessible through their props. Would that solve part of your problem?

Concerning the setter I'm not sure how that should look like but if you provide some more concrete pseudo code I can take a closer look.

growthwp commented 3 years ago

Disclaimer: I think, for what the library does, the implementation I chose is very easy to get, there's no issue here. This is more of a talk of "what could be", an idea for maybe a future rewrite (if you're planning on it) or a better data flow.

So, knowing how the library is implemented, I guess this wouldn't work, but the most elegant way for this would be to use context of some sorts. For example:

<Container context />

Where, within context there could be a key items where all the flex elements exist, identified by an id and a value, then, all of the children would have access to that context and they'd be able to read from the context, as well as update their very own values, think:

export const ReflexElement = () = > {
  const { id, flex, updateFlex } = useContext(ContainerContext);

  return(
   <div onResize={(newFlex) => updateFlex(newFlex)}>
   </div>
  );
}

Obviously, this is some pseudo-code, but you get the point. The goal of the context is so that things can talk to each other so that, at the end of it all, we have access to all that info wherever we need it, be it in the container, be in the element, of course, all somewhat "encapsulated" to the container level.

That's how most libraries choose to approach things and it seems how things are written nowadays. It's so much simpler.

However, here's how I implemented it, for the time being:

First, let's retrieve the initial flexes (in my case, from the server, then in the redux store):

  const containersIds = useSelector((state) => getRowContainersIds(state, id));
  //This is simply a function that retrieves all the containers's widths from the store.
  const containersWidths = useSelector((state) => getRowContainersWidths(state, id));

  //We're just gonna mix together the ids and their widhts to create a map where we could
  // look up any given element and retrieve its width (flex, in our case).
  const containersInfo = containersIds.map((id, i) => {
    return {
      id: containersIds[i],
      flex: containersWidths[i].width,
    };
  });

Then, let's go ahead and create our elements! We'll be providing these flex values directly to them:

  /**
   * So, the reason we're doing this kind of loop here is because `react-reflex` doesn't have
   * fragment support.
   *
   * @see https://github.com/leefsmp/Re-Flex/issues/139
   *
   * We need to go through each container info bit and then, based on that, generate an
   * array that contains both a `ReflexElement` and a `ReflexSplitter` for our resize-containers
   * functionality to work.
   */
  const containers = [];

  containersInfo.forEach(({ id: containerId, flex }, i, { length }) => {
    const isFirst = i === 0;
    const isLast = i + 1 === length;

    containers.push(
      <ReflexElement
        index={i}
        key={`row_${id}_reflexElement${i}`}
        onStopResize={(e) => handleOnResizeStop(e)}
        flex={flex}
      >
        <Container
          id={containerId}
          position={{ number: i, isFirst, isLast }}
          key={`row_${id}_container_${containerId}`}
        />
      </ReflexElement>
    );

Notice the handleOnResizeStop? Yup, when the user is done resizing, we'll push that value into the store (obviously, one could use the handleOnResize event but this is an implementation detail. In my case, I don't wanna hammer the Redux store on every resize tick since it makes no sense):

  const handleOnResizeStop = (e) => {
    const elementIndex = parseInt(e.component.key.split('reflexElement')[1]);

    dispatch(
      containerWidthUpdated({
        id: containersInfo[elementIndex].id,
        width: e.component.props.flex.toFixed(2),
      })
    );
  };

Now, because we updated the store, our containersWidths will be re-run and it will cause a re-render with the new values so that containersWidths gets updated.

And, well, that's how I got it to work. It's not a bad implementation, really. It's relatively easy to get and follow but the initial thoughts were "well, how could we make it so that you could do whatever you wanted with the library?". A possible answer is the free-flow of data, provided by using a context.

Frankly, the model where data is free-flow, accessible and open has never failed. There are quirks specifically about createContext that are easy to get and its performance penalties can easily be patched at the library-level (so people using your library don't have to worry). This approach will also yield better results: you'll be able to separate the data from the elements themselves, in essence, create a data layer and a view layer that work together but can be interacted with independently of each other.

leefsmp commented 3 years ago

Yes I agree with you, the approach of the initial implementation of the lib is getting rusty. I was thinking about a complete rewrite of the internal logic based on context and hooks. I will try to find a moment to look into it. In the meantime if you have design recommendations for that next version, I'm happy to hear it maybe in a new issue thread so it's easier to follow. Thanks for the constructive feedback.

growthwp commented 3 years ago

I am not aware of the issues you faced, the contexts in which the library is used, so, I can't say anything. However, here's an implementation of an accordion component in which all the data-flow is completely separated from the UI:

https://codesandbox.io/s/strange-leakey-1z1lz?file=/src/App.js:348-369

This doesn't use context in the way I recommended, but you can see how I have access to all the internal data of the accordion:

So what does this all achieve? Well the useAccordion hook is a barebones data manager while withAccordion deals with figuring out data one level higher: the click handlers, the content, etc. Obviously, this is not perfect design and perhaps the two things could be merged but you get the point - I get a concise place for all phases of how it works. All of this allows me to write any markup I want for my accordion. The hooks don't spit out any markup at all, as you can see, it's all handled in App.js's .map.

Composition can easily be achieved by using refs or injecting the variables that come back as props for elements, just as dnd-kit does it:

https://docs.dndkit.com/api-documentation/draggable#node-ref

Since a library needs to be batteries-included, I'd also include the presentational layer of what is App.js in my case, since people just want to import something and it to work.

Hope this at least gives you some starting points.