fix(focus-trap): update focus trap attrs to camel case #6799 #6960

amcdnl · 2017-09-09T16:01:43Z

Addresses #6799

mmalerba · 2017-09-09T19:04:57Z

src/cdk/a11y/focus-trap.ts

@@ -144,19 +144,24 @@ export class FocusTrap {
   * @param bound The boundary to get (start or end of trapped region).
   * @returns The boundary element.
   */
-  private _getRegionBoundary(bound: 'start' | 'end'): HTMLElement | null {
+  private _getRegionBoundary(bound: 'Start' | 'End'): HTMLElement | null {


Can we change this back to 'start' and 'end'? I'm pretty sure everywhere else we use string literal types we use all lowercase, we can just convert to upper case in the method

mmalerba · 2017-09-12T05:43:20Z

src/demo-app/sidenav/sidenav-demo.html

@@ -68,10 +68,10 @@
  <md-sidenav #focusSidenav>
    <md-nav-list>
      <a md-list-item routerLink>Link</a>
-      <a md-list-item routerLink cdk-focus-region-start>Focus region start</a>
+      <a md-list-item routerLink cdkFocusStart>Focus region start</a>


cdkFocusRegionStart

mmalerba · 2017-09-12T05:43:30Z

src/demo-app/sidenav/sidenav-demo.html

-      <a md-list-item routerLink cdk-focus-initial>Initially focused</a>
-      <a md-list-item routerLink cdk-focus-region-end>Focus region end</a>
+      <a md-list-item routerLink cdkFocusInitial>Initially focused</a>
+      <a md-list-item routerLink cdkFocusEnd>Focus region end</a>


cdkFocusRegionEnd

face palm * ---> fixed.

mmalerba · 2017-09-22T20:55:14Z

please rebase

…cus-trap-6799 # Conflicts: # src/demo-app/sidenav/sidenav-demo.html

amcdnl · 2017-09-24T15:01:47Z

@mmalerba - done. Noticed that the focus trap is removed from the demo now, is that by design?

mmalerba · 2017-09-24T17:09:10Z

@amcdnl it just moved to the drawer demo: https://github.com/angular/material2/blob/master/src/demo-app/drawer/drawer-demo.html

mmalerba · 2017-10-18T22:49:55Z

@amcdnl rebase?

amcdnl · 2017-10-19T15:16:28Z

@mmalerba - done :)

mmalerba · 2017-10-19T15:41:35Z

And the sidenav demo got moved to the drawer demo, so that needs update as well

amcdnl · 2017-10-20T13:56:53Z

@mmalerba - done

mmalerba · 2017-10-24T16:12:08Z

src/cdk/a11y/focus-trap.ts

    // Contains the deprecated version of selector, for temporary backwards comparability.
    let markers = this._element.querySelectorAll(`[cdk-focus-region-${bound}], ` +
-                                                 `[cdk-focus-${bound}]`) as NodeListOf<HTMLElement>;
+                                                 `[cdkFocusRegion${bound}], ` +
+                                                 `[cdk-focus-${name}]`) as NodeListOf<HTMLElement>;


name should be bound

mmalerba · 2017-10-24T16:12:14Z

src/cdk/a11y/focus-trap.ts


    for (let i = 0; i < markers.length; i++) {
      if (markers[i].hasAttribute(`cdk-focus-${bound}`)) {
        console.warn(`Found use of deprecated attribute 'cdk-focus-${bound}',` +
-                     ` use 'cdk-focus-region-${bound}' instead.`, markers[i]);
+                     ` use 'cdkFocusRegion${name}' instead.`, markers[i]);


name should be bound

mmalerba · 2017-10-24T16:12:20Z

src/cdk/a11y/focus-trap.ts

+                     ` use 'cdkFocusRegion${name}' instead.`, markers[i]);
+      } else if (markers[i].hasAttribute(`cdk-focus-region-${bound}`)) {
+        console.warn(`Found use of deprecated attribute 'cdk-focus-region-${bound}',` +
+                     ` use 'cdkFocusRegion${name}' instead.`, markers[i]);


name should be bound

amcdnl · 2017-10-24T17:23:13Z

Done :)

angular-automatic-lock-bot · 2019-09-07T12:02:26Z

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.}

fix(focus-trap): update focus trap attrs to camel case angular#6799

da9c1ea

amcdnl requested review from jelbourn and mmalerba September 9, 2017 16:01

amcdnl requested a review from devversion as a code owner September 9, 2017 16:01

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 9, 2017

mmalerba reviewed Sep 9, 2017

View reviewed changes

chore(nit): update to use lower case names

d24830d

mmalerba reviewed Sep 12, 2017

View reviewed changes

chore(nit): fix typo

d47e6c5

mmalerba added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Sep 12, 2017

jelbourn added the P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful label Sep 22, 2017

mmalerba added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Sep 22, 2017

Merge branch 'master' of https://github.com/angular/material2 into fo…

506f049

…cus-trap-6799 # Conflicts: # src/demo-app/sidenav/sidenav-demo.html

mmalerba removed the pr: needs rebase label Sep 24, 2017

Merge branch 'master' into focus-trap-6799

339042b

mmalerba added action: merge The PR is ready for merge by the caretaker and removed action: merge The PR is ready for merge by the caretaker labels Oct 19, 2017

chore(demo): update demos

df5fb46

amcdnl requested a review from josephperrott October 20, 2017 13:56

mmalerba added the action: merge The PR is ready for merge by the caretaker label Oct 20, 2017

mmalerba reviewed Oct 24, 2017

View reviewed changes

chore(nit): fix pr feedback

34e21ba

jelbourn mentioned this pull request Oct 27, 2017

Document control attributes for cdkTrapFocus #8082

Closed

mmalerba merged commit c663fad into angular:master Oct 27, 2017

amcdnl deleted the focus-trap-6799 branch October 30, 2017 14:18

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(focus-trap): update focus trap attrs to camel case #6799 #6960

fix(focus-trap): update focus trap attrs to camel case #6799 #6960

amcdnl commented Sep 9, 2017

mmalerba Sep 9, 2017

amcdnl Sep 11, 2017

mmalerba Sep 12, 2017

mmalerba Sep 12, 2017

amcdnl Sep 12, 2017

mmalerba commented Sep 22, 2017

amcdnl commented Sep 24, 2017

mmalerba commented Sep 24, 2017

mmalerba commented Oct 18, 2017

amcdnl commented Oct 19, 2017

mmalerba commented Oct 19, 2017

amcdnl commented Oct 20, 2017

mmalerba Oct 24, 2017

mmalerba Oct 24, 2017

mmalerba Oct 24, 2017

amcdnl commented Oct 24, 2017

angular-automatic-lock-bot bot commented Sep 7, 2019

fix(focus-trap): update focus trap attrs to camel case #6799 #6960

fix(focus-trap): update focus trap attrs to camel case #6799 #6960

Conversation

amcdnl commented Sep 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmalerba commented Sep 22, 2017

amcdnl commented Sep 24, 2017

mmalerba commented Sep 24, 2017

mmalerba commented Oct 18, 2017

amcdnl commented Oct 19, 2017

mmalerba commented Oct 19, 2017

amcdnl commented Oct 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amcdnl commented Oct 24, 2017

angular-automatic-lock-bot bot commented Sep 7, 2019