fix(tree, tree-item): works when tree is the topmost element in a shadow root where it has no parent #5472

Elijbet · 2022-10-13T17:52:40Z

Related Issue: #5333

Resolves #5464 as well, stems from #4814.

Summary

Adds optional chaining to get rid of the typeError in cases when there is no parentEl.

Project #adoptatree is good to go.

…hen there is no parentEl

…arent

jcfranco

This is great, @Elijbet. Aside from some test comments, this LGTM!

src/components/tree/tree.e2e.ts

jcfranco

👐👐👐👐👐👐👐
👐👐🌳👐🌳👐👐
👐🌳🌳🌳🌳🌳👐
👐👐🌳👐🌳👐👐
👐🌳🌳🌳🌳🌳👐
👐👐🌳👐🌳👐👐
👐👐👐👐👐👐👐

👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐
👐🌳🌳🌳🌳👐🌳🌳🌳👐👐👐🌳🌳👐👐🌳🌳🌳👐👐🌳🌳🌳🌳🌳👐
👐🌳👐👐🌳👐🌳👐👐🌳👐🌳👐👐🌳👐🌳👐👐🌳👐👐👐🌳👐👐👐
👐🌳🌳🌳🌳👐🌳👐👐🌳👐🌳👐👐🌳👐🌳🌳🌳👐👐👐👐🌳👐👐👐
👐🌳👐👐🌳👐🌳👐👐🌳👐🌳👐👐🌳👐🌳👐👐👐👐👐👐🌳👐👐👐
👐🌳👐👐🌳👐🌳🌳🌳👐👐👐🌳🌳👐👐🌳👐👐👐👐👐👐🌳👐👐👐
👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐

👐👐👐👐👐👐
👐🌳🌳🌳🌳👐
👐🌳👐👐🌳👐
👐🌳🌳🌳🌳👐
👐🌳👐👐🌳👐
👐🌳👐👐🌳👐
👐👐👐👐👐👐

👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐
👐🌳🌳🌳🌳🌳👐🌳🌳🌳👐👐🌳🌳🌳🌳👐🌳🌳🌳🌳👐
👐👐👐🌳👐👐👐🌳👐👐🌳👐🌳👐👐👐👐🌳👐👐👐👐
👐👐👐🌳👐👐👐🌳🌳🌳👐👐🌳🌳🌳👐👐🌳🌳🌳👐👐
👐👐👐🌳👐👐👐🌳👐🌳👐👐🌳👐👐👐👐🌳👐👐👐👐
👐👐👐🌳👐👐👐🌳👐👐🌳👐🌳🌳🌳🌳👐🌳🌳🌳🌳👐
👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐👐

…ce the bug is prone to happen after initial render

…arent

jcfranco

One more pass on the test and I think this will be good to merge!

src/components/tree/tree.e2e.ts

…arent

macandcheese · 2022-10-14T20:45:46Z

A accepted Screener - it was an unrelated new screenshot from Block changes.

This feedback has been addressed.

jcfranco

✨🌳✨

jcfranco · 2022-10-18T16:28:10Z

src/components/tree/tree.tsx

@@ -283,7 +283,7 @@ export class Tree {
      }

      // When focus is on a child node that is also either an end node or a closed node, moves focus to its parent node.
-      const parentItem = target.parentElement.closest("calcite-tree-item");
+      const parentItem = target.parentElement?.closest("calcite-tree-item");


Per our convo, this and the updates below (updateAncestorTree) wouldn't need the optional chaining operator as the event is processed after it bubbles, so we can assume there's an ancestor chain. This can be refactored later.

* feat(rating): add built-in translations * set asset dir * add initial message load * add effectiveLocale watcher * fix outdated reference * update lang switch helper to account for strings used in rendering * feat(time-picker, input-time-picker): add numberingSystem property (#5301) * feat(time-picker, input-time-picker): add numberingSystem property for localization * wip: need to correctly apply numberingSystem on InputInput * use NumberStringFormat to delocalize time * cleanup * delocalize time on blur * test(input-time-picker): fix e2e failures and add numberingSystem story * use formatter's format method instead of localize for performance * cleanup * fix(input-time-picker): don't pass through lang and numberingSystem to calcite-input * fix test * use NumberStringFormat's setter instead of setOptions Co-authored-by: Ben Elan <[email protected]> * refactor(block): move hardcoded classes to CSS object (#5474) * fix(input, input-number, input-text): fix input icons not displaying properly in Firefox (#5475) #5417 * ci(next): clean docs build before deployment (#5476) * ci(prepReleaseCommit): skip git sanity checks for next releases * ci(next): clean docs build before deployment * Revert "ci(prepReleaseCommit): skip git sanity checks for next releases" This reverts commit 7cc2d8b. Co-authored-by: Ben Elan <[email protected]> * refactor(input-number, input-text): drop resize-icon selectors (#5477) * fix(block): improve content layout (#5473) #5422 * docs: update component READMEs (#5479) Co-authored-by: jcfranco <[email protected]> * docs: add backticks to props and values for consistency (#5485) * docs: add backticks to props and values for consistency * update prop ref * review edits * fix(date-picker): no longer hides year for zh-CN locale (#5344) * fix(date-picker): no longer hides year for zn-CH locale * add screener story * fix locale typo * fix suffix placement * docs(changelog): add missing breaking changes (#5469) * feat(stepper, stepper-item): add numberingSystem property (#5467) * feat(stepper, stepper-item): add numberingSystem property * use setter for NumberStringFormat's options instead of setOptions * change formatter options in effectiveLocale watcher * cleanup Co-authored-by: Ben Elan <[email protected]> * fix(flow-item): fix scrollContentTo (#5487) #5414 * ci(next): fix next releases take 3 (#5497) * ci(next): fix next releases take 3 * cleanup Co-authored-by: Ben Elan <[email protected]> * fix(flow-item, panel): fix layout issue that would cause double scrollbars (#5486) #5428 * 1.0.0-next.598 * docs: update component READMEs (#5501) Co-authored-by: jcfranco <[email protected]> * build(deps): bump eslint-plugin-jest from 27.1.1 to 27.1.2 (#5507) Bumps [eslint-plugin-jest](https://github.com/jest-community/eslint-plugin-jest) from 27.1.1 to 27.1.2. - [Release notes](https://github.com/jest-community/eslint-plugin-jest/releases) - [Changelog](https://github.com/jest-community/eslint-plugin-jest/blob/main/CHANGELOG.md) - [Commits](jest-community/eslint-plugin-jest@v27.1.1...v27.1.2) --- updated-dependencies: - dependency-name: eslint-plugin-jest dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump shell-quote from 1.7.3 to 1.7.4 (#5503) Bumps [shell-quote](https://github.com/ljharb/shell-quote) from 1.7.3 to 1.7.4. - [Release notes](https://github.com/ljharb/shell-quote/releases) - [Changelog](https://github.com/ljharb/shell-quote/blob/main/CHANGELOG.md) - [Commits](ljharb/shell-quote@1.7.3...v1.7.4) --- updated-dependencies: - dependency-name: shell-quote dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump @typescript-eslint/eslint-plugin from 5.38.1 to 5.40.1 (#5506) Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 5.38.1 to 5.40.1. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.40.1/packages/eslint-plugin) --- updated-dependencies: - dependency-name: "@typescript-eslint/eslint-plugin" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump @typescript-eslint/parser from 5.38.1 to 5.40.1 (#5504) Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 5.38.1 to 5.40.1. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.40.1/packages/parser) --- updated-dependencies: - dependency-name: "@typescript-eslint/parser" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump eslint-plugin-jsdoc from 39.3.6 to 39.3.13 (#5505) Bumps [eslint-plugin-jsdoc](https://github.com/gajus/eslint-plugin-jsdoc) from 39.3.6 to 39.3.13. - [Release notes](https://github.com/gajus/eslint-plugin-jsdoc/releases) - [Commits](gajus/eslint-plugin-jsdoc@v39.3.6...v39.3.13) --- updated-dependencies: - dependency-name: eslint-plugin-jsdoc dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix(tree, tree-item): works when tree is the topmost element in a shadow root where it has no parent (#5472) #5333 * ci: fix next releases take 4 (#5500) * ci: fix next releases take 4 * separate error messages Co-authored-by: Ben Elan <[email protected]> * 1.0.0-next.599 * fix(flow-item): Render back button first. (#5511) * 1.0.0-next.600 * fix(tab): applies section styles onto the enclosing parent (#5516) * WIP: temorary demo swap with the issue case * fix(tab): applies section styles onto the enclosing parent * revert to original demo * cleanup * TabChilrenWithPercentageHeights story * cleanup * swap parent selector for [role=tabpanel] * Introduce a new class selector * 1.0.0-next.601 Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Ben Elan <[email protected]> Co-authored-by: Ben Elan <[email protected]> Co-authored-by: Calcite Admin <[email protected]> Co-authored-by: jcfranco <[email protected]> Co-authored-by: Kitty Hurley <[email protected]> Co-authored-by: Anveshreddy mekala <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Eliza Khachatryan <[email protected]> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Matt Driscoll <[email protected]>

Elijbet added 3 commits October 13, 2022 10:46

fix(tree, tree-item): works when tree is in the root node, in cases w…

c5e386d

…hen there is no parentEl

add more checks

2fd4874

Merge branch 'master' into elijbet/5333-update-tree-to-work-without-p…

54d679f

…arent

github-actions bot assigned Elijbet Oct 13, 2022

Elijbet changed the title ~~fix(tree, tree-item): works when tree is in the root node, in cases when there is no parentEl~~ fix(tree, tree-item): works when tree is in the root node for cases when there is no parentEl Oct 13, 2022

github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Oct 13, 2022

Elijbet added 2 commits October 13, 2022 13:41

test for renderign when tree is in the root node (there is no parent)

0b0a827

Merge branch 'master' into elijbet/5333-update-tree-to-work-without-p…

bcb1744

…arent

Elijbet marked this pull request as ready for review October 13, 2022 21:05

Elijbet requested a review from a team as a code owner October 13, 2022 21:05

Elijbet requested review from jcfranco and benelan October 13, 2022 21:07

cleanup

3396584

jcfranco approved these changes Oct 13, 2022

View reviewed changes

jcfranco reviewed Oct 13, 2022

View reviewed changes

Elijbet added 2 commits October 13, 2022 18:12

modify to assert that no error is thrown instead of using renders sin…

ba98ab8

…ce the bug is prone to happen after initial render

Merge branch 'master' into elijbet/5333-update-tree-to-work-without-p…

373c23b

…arent

Elijbet changed the title ~~fix(tree, tree-item): works when tree is in the root node for cases when there is no parentEl~~ fix(tree, tree-item): works when tree is the topmost element in a shadow root where it has no parent Oct 14, 2022

jcfranco previously requested changes Oct 14, 2022

View reviewed changes

src/components/tree/tree.e2e.ts Outdated Show resolved Hide resolved

Elijbet added 3 commits October 14, 2022 09:57

remove call as this should be captured in defineTestElement

f9105cc

Merge branch 'master' into elijbet/5333-update-tree-to-work-without-p…

742537f

…arent

Merge branch 'master' into elijbet/5333-update-tree-to-work-without-p…

b1aa7ec

…arent

Elijbet added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Oct 14, 2022

Elijbet removed the request for review from benelan October 14, 2022 22:23

tweak test

4ede605

jcfranco approved these changes Oct 18, 2022

View reviewed changes

jcfranco merged commit 8ac3785 into master Oct 18, 2022

jcfranco deleted the elijbet/5333-update-tree-to-work-without-parent branch October 18, 2022 17:26

github-actions bot added this to the Sprint 2022/10/10 - 2022/10/21 milestone Oct 18, 2022

jcfranco mentioned this pull request Oct 18, 2022

chore: remove unused import #5517

Merged

Elijbet mentioned this pull request Oct 19, 2022

Refactor to drop the optional chaining operator ? from tree within the event handler. #5520

Closed

This was referenced Oct 19, 2022

refactor(tree): drop the optional chaining operator ? from tree within the event handler #5523

Merged

Refactor with additional test for does not throw when tree-item is the topmost element in a shadow root #5527

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(tree, tree-item): works when tree is the topmost element in a shadow root where it has no parent #5472

fix(tree, tree-item): works when tree is the topmost element in a shadow root where it has no parent #5472

Uh oh!

Elijbet commented Oct 13, 2022 •

edited

Loading

Uh oh!

jcfranco left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jcfranco left a comment •

edited

Loading

Uh oh!

jcfranco left a comment

Uh oh!

Uh oh!

macandcheese commented Oct 14, 2022

Uh oh!

jcfranco left a comment

Uh oh!

jcfranco Oct 18, 2022

Uh oh!

Uh oh!

fix(tree, tree-item): works when tree is the topmost element in a shadow root where it has no parent #5472

fix(tree, tree-item): works when tree is the topmost element in a shadow root where it has no parent #5472

Uh oh!

Conversation

Elijbet commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

jcfranco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jcfranco left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcfranco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

macandcheese commented Oct 14, 2022

Uh oh!

jcfranco left a comment

Choose a reason for hiding this comment

Uh oh!

jcfranco Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Elijbet commented Oct 13, 2022 •

edited

Loading

jcfranco left a comment •

edited

Loading