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(flow): process items on loaded #11364

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

jcfranco
Copy link
Member

@jcfranco jcfranco commented Jan 23, 2025

Related Issue: #10731

Summary

This moves flow-item processing to loaded, so all items are guaranteed to be ready.

Note: items is no longer decorated with @state, as it is not used in rendering.

Verified

This commit was signed with the committer’s verified signature.
Gigioliva Gianluigi Oliva
@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jan 23, 2025
@jcfranco jcfranco added no changelog entry Use the commit override to avoid a changelog entry pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Jan 23, 2025
@jcfranco
Copy link
Member Author

@driskull @macandcheese I wasn't able to reproduce this in our E2E test environment. Do you think a screenshot test would be an acceptable form of coverage?

@driskull
Copy link
Member

I wasn't able to reproduce this in our E2E test environment. Do you think a screenshot test would be an acceptable form of coverage?

Yeah I think that should be fine.

Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

@@ -120,7 +120,6 @@ export class Flow extends LitElement implements LoadableComponent {

override connectedCallback(): void {
this.itemMutationObserver?.observe(this.el, { childList: true, subtree: true });
this.handleMutationObserverChange();
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be fine since connecting a flow doesn't need to update items. The mutation observer should suffice when items are added. If we find an issue we can call on connectedCallback and loaded.

@jcfranco jcfranco merged commit 93e5ff2 into dev Jan 23, 2025
21 checks passed
@jcfranco jcfranco deleted the jcfranco/10731-process-flow-items-when-loaded branch January 23, 2025 08:00
@jcfranco
Copy link
Member Author

I merged before I could add a screenshot test. Will do that later today.

benelan added a commit that referenced this pull request Jan 24, 2025
* origin/dev: (34 commits)
  build: update browserslist db (#11339)
  build(deps): update dependency lint-staged to v15.4.1 (#11343)
  chore: release next
  feat(graph): add component tokens (#11355)
  chore: release next
  fix(popover, tooltip): drop relative-positioning to reduce risk of clipping (#11373)
  chore: release next
  fix(date-picker): no longer disable min/max value month in select menu (#11350)
  chore(icon): improve icon load error message (#11367)
  fix(date-picker): remove outline for header actions (#11369)
  chore: release next
  fix(carousel): Ensure correct `autoplay` display and animation (#11338)
  chore: release next
  fix(flow): process items on loaded (#11364)
  chore: release next
  fix(combobox, stepper, table): respect user hidden attribute (#10983)
  refactor(action-pad): restore rounded styling (#11358)
  test(combobox): avoid emitting change event for value property update (#11281)
  chore: release next
  refactor(action-pad): remove unnecessary overflow css styling. (#11349)
  ...
benelan pushed a commit that referenced this pull request Feb 8, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports for broken functionality. Issues should include a reproduction of the bug. no changelog entry Use the commit override to avoid a changelog entry pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants