Add support for setting a default Component role

Component configuration should include an option for the default element role.

@Component({
  selector: 'md-checkbox',
  role: 'checkbox'
})
class MdCheckbox{ /* ... */ }

This is interesting, haven't thought about specifying role on the component level, which somehow makes sense. For now I've sent a PR to bind to the "role" attribute in templates #630, not sure if both the idea from this issue and template bindings are needed (I guess so).

Yeah, both are needed. #630 is about letting you bind to the role, but this is saying that a given Component should always have role X by default.

Makes sense to me. +1

I can take a stab at it. Before jumping to the impl, though, what is the advantage of expressing role in JS as opposed to having it as an attribute in a template? I guess it makes things easier if there are multiple templates, but is there more to it?

If you want to make a Component that always has a particular role, it would be annoying boilerplate to have to specify the role every time, like:

<md-button role="button">Go!</md-button>
vs.
<md-button>Go!</md-button>

Obviously this Component will always be a role="button", so the component author can save the user that boilerplate.

@jelbourn

@jelbourn sure, but what I meant is moving the role attribute to the template of a component. But now I realise it would be probably a wrong place, since the role would be marked on an element that is a child of the md-button element, right?

@pkozlowski-opensource

@pkozlowski-opensource That's right, and that would (potentially) be inside of the shadow DOM and would be invisible to screen readers.

@jelbourn

@jelbourn we should be fine with screen readers, seems like those can "break through" the shadow root barrier: http://substantial.com/blog/2014/02/05/accessibility-and-the-shadow-dom/

@mhevery @rkirov what would be the best place to add this logic? I can special case it in the DirectiveParser step, but maybe it is worth to have a dedicated pipeline step for this?

I think this would have to be a compile step. Since you want to update the template so that you don't have to do it on clone.

assume template

<div>
  <md-button></md-button>
</div>

While compiling the template we would come across md-button and we would know that it has a component. We could then see that the component has a role and update the element in template. This would be a one time thing resulting in this template:

<div>
  <md-button role="button"></md-button>
</div>

Thanks to #1052 , this is doable with @PropertySetter('attr.role'). Is it good enough or do we still need the map ?

@Mlaval

@Mlaval I think it will be good enough to move forward. There might be another issue in the future about how much boilerplate is involved with @PropertySetter

@mhevery

It is true that it can be a lot of boilerplate with @PropertySetter, @EventEmitter, the future @Static if it happens, etc.
We could add some sugar with an option in the directive's configuration, but it will for sure make the annotation more complex.

@mhevery any thoughts ?

I agree that we absolutely need some way to cut down on the boilerplate of the sheer number of things a component needs to inject. For example, the material grid-list tile would be something like

class MdGridTile {
  constructor(
      @Parent() gridList: MdGridList,
      @PropertySetter('style.height') heightSetter: Function,
      @PropertySetter('style.width') widthSetter: Function,
      @PropertySetter('style.top') topSetter: Function,
      @PropertySetter('style.left') leftSetter: Function,
      @PropertySetter('style.marginTop') marginTopSetter: Function,
      @PropertySetter('style.paddingTop') paddingTopSetter: Function,
      @PropertySetter('role') roleSetter: Function
      ) {
    this.gridList = gridList;
    this.heightSetter = heightSetter;
    this.widthSetter = widthSetter;
    this.topSetter = topSetter;
    this.leftSetter = leftSetter;
    this.marginTopSetter = marginTopSetter;
    this.paddingTopSetter = paddingTopSetter;
    roleSetter('listitem');
    // Now I can actually start doing things.
  }
}

Can't you just inject:

class MdGridTile {
  constructor(
      @Parent() gridList: MdGridList,
      @PropertySetter('style') styleSetter: Function,
      @PropertySetter('role') roleSetter: Function
      ) {
    this.gridList = gridList;
    roleSetter('listitem');
    // Now I can actually start doing things.
  }
}

This was filed before you could do @PropertySetter('attr.role'). Closing now.

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

Navigation Menu

Repository navigation

Add support for setting a default Component role #631

10 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Add support for setting a default Component role #631

Description

Activity

pkozlowski-opensource commented on Feb 13, 2015

jelbourn commented on Feb 13, 2015

mhevery commented on Feb 18, 2015

pkozlowski-opensource commented on Feb 18, 2015

jelbourn commented on Feb 18, 2015

pkozlowski-opensource commented on Feb 18, 2015

jelbourn commented on Feb 18, 2015

pkozlowski-opensource commented on Feb 20, 2015

mhevery commented on Feb 20, 2015

10 remaining items

marclaval commented on Mar 23, 2015

jelbourn commented on Mar 23, 2015

marclaval commented on Mar 25, 2015

jelbourn commented on Mar 25, 2015

mhevery commented on Mar 31, 2015

jelbourn commented on Mar 31, 2015

angular-automatic-lock-bot commented on Sep 6, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions