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(tooltip): emits close and beforeClose events when container is set to display:none #7258

Merged

Conversation

Elijbet
Copy link
Contributor

@Elijbet Elijbet commented Jun 30, 2023

Related Issue: #6279

Summary

This PR modifies onToggleOpenCloseComponent to account for the cases when open is toggled, event listeners are set up and the element is removed before the transition gets a chance to start by adding a timeout with a check of whether the transition has started.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…t to display:none
@Elijbet Elijbet added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jun 30, 2023
@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jun 30, 2023
@driskull
Copy link
Member

Since connectOpenCloseComponent and disconnectOpenCloseComponent are outdated we should update the doc for them and mark them as @deprecated as well. Just so nobody else tries to implement them in another component

@jcfranco
Copy link
Member

jcfranco commented Jul 3, 2023

It might be more scalable and perf-conscious to listen to transitioncancel than looking at the computed display value to determine the open/close 'finale' 🎩🪄. Could you give that a try?

@Elijbet
Copy link
Contributor Author

Elijbet commented Jul 3, 2023

It might be more scalable and perf-conscious to listen to transitioncancel than looking at the computed display value to determine the open/close 'finale' 🎩🪄. Could you give that a try?

Hm.. I think getting computed display was just a console.log for demo purposes.

Elijbet and others added 11 commits July 3, 2023 17:00
**Related Issue:** None

## Summary

- Depends on #7252
- When the back button is clicked, `setFocus()` is called and the newly
active panel is focused.
- Adds test
Bumps [focus-trap](https://github.com/focus-trap/focus-trap) from 7.4.3
to 7.5.2.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/focus-trap/focus-trap/releases">focus-trap's
releases</a>.</em></p>
<blockquote>
<h2>v7.5.1</h2>
<h3>Patch Changes</h3>
<ul>
<li>d9e2546: Fix possible exception in new
<code>Tabbable.getTabIndex()</code> when initializing trap</li>
</ul>
<h2>v7.5.0</h2>
<h3>Minor Changes</h3>
<ul>
<li>5e2f913: Adds support for nodes with a positive tabindex in
single-container traps only (<a
href="https://redirect.github.com/focus-trap/focus-trap/issues/375">#375</a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/focus-trap/focus-trap/blob/master/CHANGELOG.md">focus-trap's
changelog</a>.</em></p>
<blockquote>
<h2>7.5.1</h2>
<h3>Patch Changes</h3>
<ul>
<li>d9e2546: Fix possible exception in new
<code>Tabbable.getTabIndex()</code> when initializing trap</li>
</ul>
<h2>7.5.0</h2>
<h3>Minor Changes</h3>
<ul>
<li>5e2f913: Adds support for nodes with a positive tabindex in
single-container traps only (<a
href="https://redirect.github.com/focus-trap/focus-trap/issues/375">#375</a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/07eec1ec96695bef99959e67f80fc891debb7249"><code>07eec1e</code></a>
Version Packages (<a
href="https://redirect.github.com/focus-trap/focus-trap/issues/995">#995</a>)</li>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/d9e25466ab5ff1aeb7d5dceadc6b829720285812"><code>d9e2546</code></a>
Fix mostRecentlyFocusedNode being initially null causing exception in
getTabI...</li>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/72cbfa96e7b66bb16fe13d76c21d14fc6fa05da9"><code>72cbfa9</code></a>
Changes for v7.5.0 that Changesets didn't do for some reason</li>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/54ff3e0de43d55fb3afdbbb827674ed5c5425624"><code>54ff3e0</code></a>
Version Packages (<a
href="https://redirect.github.com/focus-trap/focus-trap/issues/993">#993</a>)</li>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/b9225d65b8bc39e349dadd73d6dcb491039732d2"><code>b9225d6</code></a>
add DaviDevMod as a contributor for code, and bug (<a
href="https://redirect.github.com/focus-trap/focus-trap/issues/992">#992</a>)</li>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/5e2f913b4a4988c813cc51427b1a2b957c50ac15"><code>5e2f913</code></a>
Support elements with positive tabindex attributes in single-container
traps ...</li>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/bc4ac04fda6653bf294640429e55209562e2c340"><code>bc4ac04</code></a>
[DEPENDABOT]: Bump <code>@​changesets/cli</code> from 2.26.1 to 2.26.2
(<a
href="https://redirect.github.com/focus-trap/focus-trap/issues/988">#988</a>)</li>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/1b6694e2e7cbfbb9f43a3b91b1bd0f0d62330b34"><code>1b6694e</code></a>
[DEPENDABOT]: Bump typescript from 5.1.3 to 5.1.5 (<a
href="https://redirect.github.com/focus-trap/focus-trap/issues/989">#989</a>)</li>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/085db4ac004c8f50fbbfc1897c4eceb673a3c0a6"><code>085db4a</code></a>
[DEPENDABOT]: Bump cypress from 12.15.0 to 12.16.0 (<a
href="https://redirect.github.com/focus-trap/focus-trap/issues/990">#990</a>)</li>
<li><a
href="https://github.com/focus-trap/focus-trap/commit/a4b3948c7fce410841dc247227cbaa1292d2efcb"><code>a4b3948</code></a>
[DEPENDABOT]: Bump eslint-plugin-jest from 27.2.1 to 27.2.2 (<a
href="https://redirect.github.com/focus-trap/focus-trap/issues/983">#983</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/focus-trap/focus-trap/compare/v7.4.3...v7.5.1">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=focus-trap&package-manager=npm_and_yarn&previous-version=7.4.3&new-version=7.5.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Matt Driscoll <mdriscoll@esri.com>
**Related Issue:** None

## Summary

- renames private method from `handlefocusTrapDisabled` to
`handleFocusTrapDisabled`
**Related Issue:** None

## Summary

- Updates all `setFocus()` methods to use `componentFocusable()` instead
of `componentLoaded()`.
- `TimePicker` component also updates in the `focusPart` private method
to use `componentFocusable()`.
- `TextArea` uses `componentLoaded()` outside of the `setFocus()`
method. This was in the `selectText()` and `resizeObserver`. I left
those them alone since they are not doing any focusing specifically.
- Added `await page.waitForChanges();` to tests that required it.
  - action-menu
  - split-button
  - commonTests helper (removed setTimeout of 0 as well)
@github-actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Jul 18, 2023
@Elijbet Elijbet removed the Stale Issues or pull requests that have not had recent activity. label Jul 20, 2023
@github-actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@Elijbet Elijbet marked this pull request as ready for review August 3, 2023 00:53
@Elijbet Elijbet requested a review from a team as a code owner August 3, 2023 00:53
@Elijbet
Copy link
Contributor Author

Elijbet commented Aug 3, 2023

@jcfranco This seems to be working as expected after the #7422 fix is implemented.

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

🙈🗣️

@jcfranco jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Aug 3, 2023
@jcfranco jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 3, 2023
@jcfranco jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 3, 2023
@jcfranco jcfranco merged commit 60a4683 into main Aug 3, 2023
@jcfranco jcfranco deleted the elijbet/6279-emit-close-and-before-close-when-dipslay-none branch August 3, 2023 18:05
@github-actions github-actions bot added this to the 2023 July Priorities milestone Aug 3, 2023
benelan added a commit that referenced this pull request Aug 3, 2023
* origin/main: (40 commits)
  fix(combobox): add space after grouped items (#7302)
  fix(tooltip): emits `close` and `beforeClose` events when container is set to `display:none` (#7258)
  chore: release next
  feat(block): improve block's content layout to allow scrolling (#7367)
  test(input, input-number): await on missed `page.waitForChanges` calls (#7429)
  chore: release next
  fix(color-picker): draw slider thumbs within bounds (#7398)
  chore: release next
  fix(slider): prevent excessive tick rendering (#7421)
  fix(tooltip): avoid extra before open/close event emitting (#7422)
  chore(storybook): hide warning and show TestOnly stories in local builds (#7424)
  chore: release next
  fix(panel): Remove double border styling when content isn't provided (#7368)
  feat(list): Add support for dragging items. (#7109)
  chore: release next
  fix(scrim): update loader scale on resize of component. (#7419)
  feat(text-area): provide additional context for AT users when character limit exceeds (#7412)
  chore: release next
  fix(chip): disconnect mutation observer when component is disconnected from the DOM (#7418)
  fix(switch): Fix for focus outline style in certain cases (#7414)
  ...
benelan pushed a commit that referenced this pull request Aug 3, 2023
🤖 I have created a release *beep* *boop*
---


<details><summary>@esri/calcite-components: 1.5.0</summary>

##
[1.5.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@1.4.3...@esri/calcite-components@1.5.0)
(2023-08-03)


### Features

* **action-group:** Adds overlayPositioning property.
([#7366](#7366))
([ca9f35a](ca9f35a))
* Allow sharing focus trap stacks via configuration global
([#7334](#7334))
([934a19f](934a19f))
* Automatically import and define Calcite Components when importing
their React wrapper
([#7185](#7185))
([bf0ff67](bf0ff67))
* **block, block-section:** Add setFocus method
([#7208](#7208))
([35d4bbb](35d4bbb))
* **block:** Improve block's content layout to allow scrolling
([#7367](#7367))
([ecbf17b](ecbf17b))
* **color-picker:** Replaces thumb focus outline to rounded
([#7378](#7378))
([d803980](d803980))
* **filter:** Add filter method
([#7127](#7127))
([5a4283f](5a4283f))
* **flow:** Adds setFocus method
([#7252](#7252))
([2472c58](2472c58))
* Improve focus behavior in components
([#7277](#7277))
([ad9fbca](ad9fbca))
* **input-time-zone:** Add input-time-zone component
([#6947](#6947))
([87bd496](87bd496))
* **list:** Add slots for filter actions
([#7183](#7183))
([da07ab1](da07ab1))
* **list:** Add support for dragging items.
([#7109](#7109))
([7324f70](7324f70))
* **menu-item:** Update spacing and icon layout
([#7381](#7381))
([5659671](5659671))
* **navigation-logo:** Increase font-size of heading with no description
([#7081](#7081))
([355e101](355e101))
* **switch:** Updates focus outline to be rounded
([#7390](#7390))
([2616b82](2616b82))
* **text-area:** Provide additional context for AT users when character
limit exceeds
([#7299](#7299))
([c5678eb](c5678eb))
* **text-area:** Provide additional context for AT users when character
limit exceeds
([#7412](#7412))
([c1af3c7](c1af3c7))


### Bug Fixes

* **accordion, accordion-item:** `icon-position`, `icon-type`,
`selection-mode` and `scale` can now be set as props or attributes
([#7191](#7191))
([2b09aba](2b09aba))
* **action-bar:** No longer delegates focus when clicked on
non-focusable region
([#7310](#7310))
([1a9c15c](1a9c15c))
* **action:** Correctly focus the button after rendering updates.
([#7255](#7255))
([40fe2ce](40fe2ce))
* **block:** Loader now appears for all loading cases
([#7303](#7303))
([5af3600](5af3600))
* **block:** Removes extra loading indicator
([#7239](#7239))
([a334a75](a334a75))
* **card:** Ensure teardown logic is called when disconnected
([#7289](#7289))
([d07e322](d07e322))
* **chip:** Disconnect mutation observer when component is disconnected
from the DOM
([#7418](#7418))
([412e5fb](412e5fb))
* **color-picker:** Draw slider thumbs within bounds
([#7398](#7398))
([2f37854](2f37854))
* **color-picker:** Fix opacity slider keyboard nudging
([#7400](#7400))
([2b4f7c3](2b4f7c3))
* **color-picker:** Maintains correct numbering system when entering
invalid RGB value
([#7327](#7327))
([8d2a3a5](8d2a3a5))
* **combobox:** Add space after grouped items
([#7302](#7302))
([b1580c7](b1580c7))
* **dropdown-item:** Provides accessible label when href is not parsed
([#7316](#7316))
([966b83d](966b83d))
* **flow:** Call setFocus() on back button click
([#7285](#7285))
([9102aa4](9102aa4))
* **input-date-picker:** Provides placeholder text context for AT users
([#7320](#7320))
([31e0ba2](31e0ba2))
* **input-date-picker:** Reset active date picker date after closing
([#7219](#7219))
([91b2a1b](91b2a1b))
* **input, input-number:** No longer removes trailing decimal separator
([#7159](#7159))
([01535cf](01535cf))
* **link:** Adds outline-offset to avoid overlapping with text.
([#7342](#7342))
([c30db4e](c30db4e))
* **list:** Changing filterText property will now update filtered items
([#7133](#7133))
([a9c0bce](a9c0bce))
* **list:** Fix keyboard navigation after a listItem's disabled or
closed property changes
([#7275](#7275))
([91d28eb](91d28eb))
* **list:** Fix keyboard navigation when filterEnabled is true
([#7385](#7385))
([41a2e42](41a2e42))
* **menu-item:** Prevent duplicate border in nested vertical menu-items
([#7387](#7387))
([186a738](186a738))
* **panel:** Remove double border styling when content isn't provided
([#7368](#7368))
([91a0610](91a0610))
* Remove style modifying all host components with hidden attribute
([#7346](#7346))
([3103e2f](3103e2f))
* **scrim:** Update loader scale on resize of component.
([#7419](#7419))
([24e7f70](24e7f70))
* **slider:** Prevent excessive tick rendering
([#7421](#7421))
([c799409](c799409))
* **switch:** Fix for focus outline style in certain cases
([#7414](#7414))
([217324f](217324f))
* **tab-title:** Add full focus outline to closable tab button in high
contrast mode
([#7272](#7272))
([d812d17](d812d17))
* **tooltip:** Avoid extra before open/close event emitting
([#7422](#7422))
([dbb6818](dbb6818))
* **tooltip:** Deprecate the label property due to the description
coming from the component's content
([#7247](#7247))
([7934d75](7934d75))
* **tooltip:** Emits `close` and `beforeClose` events when container is
set to `display:none`
([#7258](#7258))
([60a4683](60a4683))
* **tooltip:** Ensure --calcite-app-z-index-tooltip is applied
([#7345](#7345))
([a9a7072](a9a7072))
</details>

<details><summary>@esri/calcite-components-react: 1.5.0</summary>

##
[1.5.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@1.4.3...@esri/calcite-components-react@1.5.0)
(2023-08-03)


### Features

* Automatically import and define Calcite Components when importing
their React wrapper
([#7185](#7185))
([bf0ff67](bf0ff67))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @esri/calcite-components bumped from ^1.5.0-next.38 to ^1.5.0
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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. 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

3 participants