Skip to content

Use lightweight token for tab group and select trigger#19728

Merged
devversion merged 4 commits intoangular:lightweight-token-optimizationsfrom
devversion:wip/light-weight-tokens-13
Jun 23, 2020
Merged

Use lightweight token for tab group and select trigger#19728
devversion merged 4 commits intoangular:lightweight-token-optimizationsfrom
devversion:wip/light-weight-tokens-13

Conversation

@devversion
Copy link
Member

  • See individual commits.

This is the last set of changes for lightweight tokens. I would consider the one for the select trigger a micro-optimization and I don't feel too confident about it, but I it's low-effort to use it there as a custom select trigger is used rarely (compared to using the standard one) I assume.

@devversion devversion added P2 The issue is important to a large percentage of users, with a workaround merge safe labels Jun 23, 2020
@devversion devversion requested a review from a team June 23, 2020 14:44
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 23, 2020
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto crisbeto added the lgtm label Jun 23, 2020
@devversion devversion force-pushed the wip/light-weight-tokens-13 branch from 719ae28 to befe0ca Compare June 23, 2020 15:10
The Angular Material tab group currently always retains `MatTabContent`
and `MatTabLabel`. These are optional features of the tabs module and
should only be retained when needed.

To achieve this, we use lightweight tokens instead of the actual
implementations when querying for a lazy content or label.

This signifies an improvement of ~500b in the common case, and
an increase (due to the lightweight tokens being provided) in
the advanced case of ~140b.

Related to angular#19576.
@devversion devversion force-pushed the wip/light-weight-tokens-13 branch from befe0ca to 41e3844 Compare June 23, 2020 15:17
The select size test has slightly shifted already due to the recent
Angular update. We are updating the golden here so that we can see
the lightweight token effect in the followed commit for select.
The Angular Material always retains the `MatSelectTrigger`
class. This is not ideal as this is an optional part
of the select. The class itself is not large enough
to contribute to any noticeable size impact, even with
Ivy where factories/definitions are directly attached, but
for the sake of completeness, and because custom triggers
are assumed to be uncommon, a lightweight token is used for
a small micro-optimization in the common case.

Related to angular#19576.
@devversion devversion force-pushed the wip/light-weight-tokens-13 branch from 41e3844 to 1d70b9c Compare June 23, 2020 15:20
Copy link
Contributor

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Jun 23, 2020
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM, for dev-infra

@devversion devversion merged commit c263ccb into angular:lightweight-token-optimizations Jun 23, 2020
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants