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

Added activeAccordion method to Accordion.php #2939

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bald-cat
Copy link
Contributor

@bald-cat bald-cat commented Jan 22, 2025

Fixes #

Proposed Changes

  • Added open method to Accordion.php. Allows you to specify which accordions should be open when the page is opened. Accepts a string and an array, if the method is not called, the first accordion will be opened by default, as it was before adding the method

? array_merge($this->variables['open'], Arr::wrap($activeAccordion))
: $activeAccordion;

$this->openSet = true;
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of the openSet property in our case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When creating a class, we set a standard key for the first tab in the constructor to preserve the standard previous behavior.

A flag property is needed so that when the method is first called, it can be distinguished whether it was called earlier and, depending on this, merge the array or overwrite it.

/**
* @var array
*/
protected $variables = [
'stayOpen' => false,
'open' => [],
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should aim for more consistency with naming? Since we're calling the method active*, perhaps it would make sense to also use the key as active.

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

Successfully merging this pull request may close these issues.

2 participants