Enable programmatic close on dropdown component

[alex-ju]

📌 Summary

Enable programmatic close on dropdown component.

🛠️ Detailed description

Based on feedback from @jgwhite we realised that we're not exposing a way for developers to close a dropdown component. In this PR we start by renaming the deactivate function in disclosure utility component to close then forward it to the dropdown component.

A simplified example of a call would look like this:

<Hds::Dropdown id="test-dropdown" as |dd|>
  <dd.ToggleButton @text="toggle button" id="test-toggle-button" />
  <dd.Interactive @route="components.dropdown" @text="interactive" id="test-list-item-interactive" {{on "click" dd.close}} />
</Hds::Dropdown>

🔗 External links

Jira issue

---

👀 How to review

👉 Review commit-by-commit

Reviewer's checklist:

• +1 Percy if applicable
• Confirm that PR has a changelog update via Changesets if needed

💬 Please consider using conventional comments when reviewing this PR.

[changeset-bot]

🦋 Changeset detected

Latest commit: 899dec3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@hashicorp/design-system-components	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
hds-components	✅ Ready (Inspect)	Visit Preview	Jun 22, 2022 at 11:45AM (UTC)
hds-flight-website	✅ Ready (Inspect)	Visit Preview	Jun 22, 2022 at 11:45AM (UTC)

[didoo]

@alex-ju added a couple of comments related to the documentation (the other code is OK, actually very nice implementation and refactoring).

[didoo]

[potential issue] what if it doesn't exist anymore, because in the meantime the page has navigated away? probably to avoid this risk in the render above (line 100) instead of a route to a different "page" I would use a @href="#"? (similar to what you did in the other test)

PS: if you want to test the actual behaviour you can

• add a line after line 106, with this code: await pauseTest(); (the method needs to be imported from @ember/test-helpers)
• launch the tests via ember test --server --filter 'Integration | Component | hds/dropdown/index' - this will launch a new browser that renders the content, and pause the test wherever you put the pauseTest() call, and you can inspect the DOM and see what it does

(FYI I did the test, and strangely enough it shows both the dropdown and the content of the target webpage, so not sure how to interpret this).

[alex-ju]

Fair concern. I forgot the trigger in this example is not a simple HTML button element, but an interactive component with a set route which renders as a link.

[didoo]

[nitpick] probably in order to be able to see that actually closes the dropdown, it would be better to add target="_blank" to the link (otherwise it opens in the same window and everything disappears). Another option would be to have a <button> instead of a link, that does nothing more that closing the disclosure?

[alex-ju]

I tested it locally with command+click so I preserve the current page, but yes, button would be more suitable. Same for integration testing.

[didoo]

[nitpick] same as above: this reloads the page, so there is no way to check that actually works. If you remove the @route it will render a <button> and when you click it you see that it closes.

[alex-ju]

@alex-ju added a couple of comments related to the documentation (the other code is OK, actually very nice implementation and refactoring).

Good points about using buttons for clarity in examples and accuracy in tests. I've updated based on feedback and it's good for a 2nd review.

[didoo]

Very nice!

[jgwhite]

Thanks for this!

[jgwhite]

I just discovered a fun little footgun related to this feature that others may run into…

So, given the use case from the description:

<Hds::Dropdown id="test-dropdown" as |dd|>
  <dd.ToggleButton @text="toggle button" />
  <dd.Interactive
    @route="components.dropdown"
    @text="interactive"
    {{on "click" dd.close}}
  />
</Hds::Dropdown>

My assumption was that {{on "click" dd.close}} is added after whatever listeners <LinkTo> adds implicitly, allowing Ember to do its normal routing stuff first, then closing the dropdown after.

Unfortunately, this was a faulty assumption. A peek at link-to.hbs reveals the truth:

<a
  {{!-- for compatibility --}}
  id={{this.id}}
  class={{this.class}}

  {{!-- deprecated attribute bindings --}}
  role={{this.role}}
  title={{this.title}}
  rel={{this.rel}}
  tabindex={{this.tabindex}}
  target={{this.target}}

  ...attributes  <---------------+
                                 |
  href={{this.href}}             | OH MY
                                 |
  {{on 'click' this.click}}  <---+
>{{yield}}</a>

Splattributes are applied before the click handler 😬 Which means dd.close is called first, which in turn means the element and all event handlers are removed before we ever get to that implicit click handler 🤦‍♀️

Funnily enough, the result almost looks like it’s working, because we still have a perfectly functional href, so it’s easy not to notice the browser is doing a full page load.

The solution is to schedule the dd.close for the next event cycle, something like this:

{{! my-component.hbs}}

<Hds::Dropdown id="test-dropdown" as |dd|>
  <dd.ToggleButton @text="toggle button" />
  <dd.Interactive
    @route="components.dropdown"
    @text="interactive"
    {{on "click" (fn this.later dd.close)}}
  />
</Hds::Dropdown>

// my-component.js

export default class extends Component {

  @action
  later(fn) {
    // Runs fn in the next event cycle (after sync listeners)
    setTimeout(fn);
  }

}