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

fix(ListGroup): don't open on external changes #231

Merged
merged 2 commits into from
Mar 24, 2023
Merged

Conversation

marstamm
Copy link
Contributor

Found while working on camunda/example-data-properties-provider#1, cf. https://camunda.slack.com/archives/GP70M0J6M/p1679407286325119

This ensures that we only open the section when the change originated from the add button click:

Recording 2023-03-22 at 14 01 11

@marstamm marstamm requested a review from a team March 22, 2023 13:02
@marstamm marstamm self-assigned this Mar 22, 2023
@marstamm marstamm requested review from philippfromme and barmac and removed request for a team March 22, 2023 13:02
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Mar 22, 2023
// open if not open and configured
if (!open && shouldOpen) {
// open if not open, configured and triggered by add button
if (addTriggered && !open && shouldOpen) {
Copy link
Member

Choose a reason for hiding this comment

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

Feels really like an awkward hack. Let's document it like that and add a // TODO(marstamm): ....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a todo and a bit of documentation to move the logic out of the ListGroup

Copy link
Member

@nikku nikku Mar 23, 2023

Choose a reason for hiding this comment

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

Thanks @marstamm.

With these comments (and this commit as a reference) the changes look manageable.

Let's try to keep our hacks per LOC as low as possible.

image

image

@marstamm marstamm force-pushed the fix-listGroup-opening branch from 17ad378 to e10d8c1 Compare March 23, 2023 15:59
@marstamm marstamm merged commit 197d5df into main Mar 24, 2023
@marstamm marstamm deleted the fix-listGroup-opening branch March 24, 2023 12:04
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Mar 24, 2023
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.

2 participants