refactor: allow options to be passed to focus methods

[crisbeto]

Updates the focus methods on the different components to allow for the FocusOptions to be passed in.

Fixes #14182.

Note: this doesn't include a few of the classes that implement FocusableOption, because it has a different requirement for the first param.

[mmalerba]

Should we make something like:

interface MatFocusOptions extends FocusOptions {
  origin: FocusOrigin?;
}

and then:

/** @breaking-change 9.0.0 remove FocusOrigin as a param type. */
focus(options: FocusOptions | FocusOrigin = {origin: 'program'}) {...}

[crisbeto]

Makes sense. I'll apply it to the cases that implement FocusableOption.

[crisbeto]

@mmalerba I'm giving this a shot in the step header, it looks like it'll add a fair bit of a code if we want to support it in a backwards-compatible way. In order support the user passing in a FocusOrigin, we'd also have to add an optional dependency on the FocusMonitor. E.g. here's what the step header would look like:

focus(config: FocusableOptionConfig = {origin: 'program'}) {
  let origin: FocusOrigin | undefined;
  let options: FocusOptions | undefined;

  if (config == null || typeof config === 'string') {
    origin = config;
  } else {
    origin = config.origin;
    options = config;
  }

  if (this._focusMonitor) {
    this._focusMonitor.focusVia(this._elementRef, origin, options);
  } else {
    this._elementRef.nativeElement.focus(options);
  }
}

[mmalerba]

@jelbourn do you have any thoughts? The extra code isn't great, but I also don't like how we wind up with inconsistent APIs otherwise

[jelbourn]

I don't quite follow. Why would you need a dependency on FocusMonitor in places where it isn't used now?

Also, FWIW, I've been considering dropping FocusMonitor entirely and always showing focus state all the time (recommended by the a11y team and what MDC Web does).

[crisbeto]

@jelbourn it's mostly for consistency.

[jelbourn]

I don't think it's right to add the FocusMonitor dependency just for that consistency. I'd rather have the discussion on whether we want to drop FocusMonitor now and then maybe just switch everything to the standard options for v8.

[mmalerba]

Ok, then this looks good for now and we can deprecate the origin param later if needed

[jelbourn]

It looks like FocusOption isn't imported in this file. It's causing a presubmit failure. I don't understand why CircleCI is green, though

[crisbeto]

There's something in the DOM API that's called the same. I think we added ours since the interface isn't there in earlier TS versions (which probably happened in G3). I've added the import.

[ngbot]

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

[ngbot]

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

[ngbot]

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

[andrewseguin]

This change means that users can no longer create a focus key manager with a query list of MatOption since MatOption no longer matches the FocusableOption interface.

[crisbeto]

Reworked to get MatOption to match FocusableOption.

[andrewseguin]

Looks like MatButton and MatCheckbox also needs to be adjusted so that they match the FocusableOption interface

[crisbeto]

Updated to have MatCheckbox and MatButton match FocusableOption.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}