Skip to content

OBPIH-6023 Add reusable Section and Subsection components#4491

Merged
awalkowiak merged 2 commits intofeature/product-supplier-list-redesignfrom
OBPIH-6023
Feb 6, 2024
Merged

OBPIH-6023 Add reusable Section and Subsection components#4491
awalkowiak merged 2 commits intofeature/product-supplier-list-redesignfrom
OBPIH-6023

Conversation

@kchelstowski
Copy link
Collaborator

Peek 2024-02-05 14-24

Comment on lines 24 to 43
<div className="v2-subsection">
<div className="subsection-title-wrapper">
<span
role="button"
tabIndex={0}
onClick={collapsable ? () => triggerCollapse() : null}
onKeyDown={collapsable ? () => triggerCollapse() : null}
style={collapsable ? { cursor: 'pointer' } : { cursor: 'unset' }}
>
<Translate id={title.label} defaultMessage={title.defaultMessage} />
{collapsable
&& (expanded
? <RiArrowUpSLine className="arrow-up" />
: <RiArrowDownSLine className="arrow-down" />)}
</span>
</div>
<div className={`subsection-body ${expanded ? 'subsection-body-expanded' : ''}`}>
{children}
</div>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is possible to reduce the amount of JS code inside the HTML...
Maybe you can create an object:

{
collapsable: {
onClick: ....,
onKeyDown: ...,
style: ...,
},
...
}

and spread it on this span?
I am just thinking out loud, you can skip it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer implicit attributes (I've explained that recently), and moreover - the ESLint would shout about that.

<Translate id={title.label} defaultMessage={title.defaultMessage} />
{collapsable
&& (expanded
? <RiArrowUpSLine className="arrow-up" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be a single csv with a class that rotates it when it is active

<RiArrowUpSLine className={`arrow-up {expanded ? 'arrow-up--expanded' : ''}`} />
.arrow-up--expanded {
  transform: rotate(180deg);
}

Comment on lines 8 to 21
const Subsection = ({
title,
collapsable,
children,
}) => {
// If a subsection is not collapsable, it is always expanded
// (collapsable: false --> expanded: true)
// If a subsection is collapsable, it is not expanded by default
// (collapsable: true --> expanded: false)
const [expanded, setExpanded] = useState(!collapsable);

const triggerCollapse = () => {
setExpanded(!expanded);
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of a single collapsable prop I would also add isExpanded: boolean as an initial value, that way we can control if the section is collapsed or not for both states when it is collapsable or not collapsable, so something like:

const Subsection = ({
  title,
  isExpanded,
  collapsable,
  children,
}) => ...

...
Subsection.defaultProps = {
  isExpanded: true,
  collapsable: true,
};
const [expanded, setExpanded] = useState(isExpanded);

prevent any toggle action if section is not collapsible, maybe name disabled fro this prop sound better but collapsable should also be good.

const triggerCollapse = () => {
  if (collapsable) {
   setExpanded(!expanded);
  }
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we are fine, because the onClick method would be null if a subsection is not collapseable, but I will add it just in case we would refactor something in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added additional prop expandedByDefault as you suggested, to make it more reusable in different scenarios. I didn't decide to go for isExpanded, not to mix isExpanded with expanded, as it might cause confusions.

@awalkowiak awalkowiak merged commit 3cfc162 into feature/product-supplier-list-redesign Feb 6, 2024
@awalkowiak awalkowiak deleted the OBPIH-6023 branch February 6, 2024 15:13
awalkowiak pushed a commit that referenced this pull request Mar 12, 2024
* OBPIH-6023 Add reusable Section and Subsection components

* OBPIH-6023 Fixes after review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants