quantum icon indicating copy to clipboard operation
quantum copied to clipboard

Row / Col components requiring key

Open LucasFlaquer opened this issue 2 years ago • 3 comments

Describe the bug

When using the grid components <Row /> and <Col /> always trhows a warning on the console. Aparently the way it's rendered is using a map, would it be possible to fix it turning those components simple divs with the css applied? image

A clear and concise description of what the bug is.

Steps to reproduce the behavior

  1. create a component with one <Row /> add two columns,
  2. Open DevTools
  3. It' will trigger the error on the console

Expected vs actual behavior

expected to render cols that are not on a loop without the need of a key prop

LucasFlaquer avatar Jul 26 '23 14:07 LucasFlaquer

I didn't quite understand the proposed solution, but I will present the current scenario and the possible solution to the problem.

Current scenario This warning is shown because the Row component clones all children and displays them using .map, regardless of whether the children are a Col or a div component. The clone of the children aims to pass the ''no-gutters'' prop of the Row component to the children that are Col components, since only the Col component benefits from the 'no-gutters' prop. The Col component uses the prop "no-gutters" to apply spacing in browsers that don't support display:grid.

Alternative to solve the problem To remove the need for the 'key' prop on the children of Row and preserve the behavior of the Col component, we need to render the children of Row directly (without making a clone and using a .map) and override the styling of the Col children from the Row component, as shown below.

import { Col } from '@quantum/Grid'; const RowStyled = styled.div´ ... ${Col} { ... } ´; ... const Row = (children) => <RowStyled>{children}</RowStyled>);

However, this approach will enlarge the Row component's .css as it will bring the Col component's style to it, increase the Row component's size because of the Col component import, and finally, will not allow the 'no' prop. -gutters' of the Col children takes precedence over the 'no-gutters' prop of the parent component, i.e. the Row component. So I don't see how beneficial it is to fix this "bug" at this time.

Is the proposed solution in any way related to the alternative mentioned above? Can you give me more details of your possible solution?

MarcosViniciusPC avatar Aug 09 '23 15:08 MarcosViniciusPC

The solution I have propoused have been made without the knologe of te structure of the component. Taking a look inside the code of the <Row /> component I've noticed that it's using a legacy structure (class component) with the cloneElement method. I undestand that you nedd to apply the no-gutthers in the chidlren elements, so, the best solution to this problem here is a refactoring of this legacy structures.

  • Turn the class component into a functional component
  • use this guide to update the cloneElement

this could be a big refactoring, however will resolve all the warnings that came from the component and will update the library

LucasFlaquer avatar Aug 14 '23 14:08 LucasFlaquer

I will put a task in the backlog to investigate the proposed solution, thank you!

MarcosViniciusPC avatar Jan 19 '24 12:01 MarcosViniciusPC