Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

[Select] foundation init may cause style recalculation unnecessarily #5686

Closed
e111077 opened this issue Mar 5, 2020 · 0 comments · Fixed by #5870
Closed

[Select] foundation init may cause style recalculation unnecessarily #5686

e111077 opened this issue Mar 5, 2020 · 0 comments · Fixed by #5870
Assignees
Labels

Comments

@e111077
Copy link

e111077 commented Mar 5, 2020

Bug report

Select, when initialized with a value, causes two style recalculations which are expensive. This is especially bad since it is on init which affects first render

Steps to reproduce

  1. initialize a select with a predefined value
  2. load up the page in chrome
  3. open dev tools to the performance tab
  4. refresh the page with the performance refresh tool
  5. see how it calls notchOutline twice

Actual behavior

MDCSelectFoundation.notchOutline is being called twice on init

Expected behavior

MDCSelectFoundation.notchOutline should only be called once on init

Screenshots

image

Your Environment:

Software Version(s)
MDC Web canary
Browser chrome
Operating System MacOS

Additional context

MWC implementation

Possible solution

add an else-if statement before the updateLabel call in init where the condition meets whether it was called via setValue

@e111077 e111077 added the bug label Mar 5, 2020
copybara-service bot pushed a commit that referenced this issue Apr 29, 2020
…to options

- Method `layout` and private method `updateLabel` have been collapsed into one `layout` method. The reasoning is partially to reduce redundancy, but also since there's no reason they should be updated separately. The old layout method may have simply been broken since it doesn't update the label, just the notch, but lack of usage meant this problem didn't surface.
- `layoutOptions` method can now be called by the client whenever the underlying list elements change (i.e. dynamic server fetch), to ensure that the foundation's state is sync'ed up. When typeahead lands in list foundation, this will also allow an optional call to list's layout method to reinitialize typeahead cache.
- The empty "" option is now properly accounted for as an option. Previously it was ignored, so if you initialized the select with "" having the selected class, it wouldn't be sync'ed to the internal state. This meant that selecting a different option via select.value="blah" wouldn't remove the "selected" classes and attributes on the "" option, meaning it would still act like "" is selected even though selected text shows up as "blah".
- `foundation.init` is no longer called twice: once by base component.ts and once by select component.ts
- `foundation.init` no longer calls `notchOutline` 3 (???) times (closes #5686)
- `foundation.init` no longer emits change event (closes #5783).

PiperOrigin-RevId: 308892334
copybara-service bot pushed a commit that referenced this issue Apr 29, 2020
…to options

- Method `layout` and private method `updateLabel` have been collapsed into one `layout` method. The reasoning is partially to reduce redundancy, but also since there's no reason they should be updated separately. The old layout method may have simply been broken since it doesn't update the label, just the notch, but lack of usage meant this problem didn't surface.
- `layoutOptions` method can now be called by the client whenever the underlying list elements change (i.e. dynamic server fetch), to ensure that the foundation's state is sync'ed up. When typeahead lands in list foundation, this will also allow an optional call to list's layout method to reinitialize typeahead cache.
- The empty "" option is now properly accounted for as an option. Previously it was ignored, so if you initialized the select with "" having the selected class, it wouldn't be sync'ed to the internal state. This meant that selecting a different option via select.value="blah" wouldn't remove the "selected" classes and attributes on the "" option, meaning it would still act like "" is selected even though selected text shows up as "blah".
- `foundation.init` is no longer called twice: once by base component.ts and once by select component.ts
- `foundation.init` no longer calls `notchOutline` 3 (???) times (closes #5686)
- `foundation.init` no longer emits change event (closes #5783).

PiperOrigin-RevId: 308892334
@allan-chen allan-chen self-assigned this Apr 29, 2020
copybara-service bot pushed a commit that referenced this issue Apr 29, 2020
…to options

- Method `layout` and private method `updateLabel` have been collapsed into one `layout` method. The reasoning is partially to reduce redundancy, but also since there's no reason they should be updated separately. The old layout method may have simply been broken since it doesn't update the label, just the notch, but lack of usage meant this problem didn't surface.
- `layoutOptions` method can now be called by the client whenever the underlying list elements change (i.e. dynamic server fetch), to ensure that the foundation's state is sync'ed up. When typeahead lands in list foundation, this will also allow an optional call to list's layout method to reinitialize typeahead cache.
- The empty "" option is now properly accounted for as an option. Previously it was ignored, so if you initialized the select with "" having the selected class, it wouldn't be sync'ed to the internal state. This meant that selecting a different option via select.value="blah" wouldn't remove the "selected" classes and attributes on the "" option, meaning it would still act like "" is selected even though selected text shows up as "blah". (closes #5646)
- `foundation.init` is no longer called twice: once by base component.ts and once by select component.ts
- `foundation.init` no longer calls `notchOutline` 3 (???) times (closes #5686)
- `foundation.init` no longer emits change event (closes #5783).

PiperOrigin-RevId: 308892334
copybara-service bot pushed a commit that referenced this issue Apr 29, 2020
…to options

- Method `layout` and private method `updateLabel` have been collapsed into one `layout` method. The reasoning is partially to reduce redundancy, but also since there's no reason they should be updated separately. The old layout method may have simply been broken since it doesn't update the label, just the notch, but lack of usage meant this problem didn't surface.
- `layoutOptions` method can now be called by the client whenever the underlying list elements change (i.e. dynamic server fetch), to ensure that the foundation's state is sync'ed up. When typeahead lands in list foundation, this will also allow an optional call to list's layout method to reinitialize typeahead cache. (closes #5646)
- The empty "" option is now properly accounted for as an option. Previously it was ignored, so if you initialized the select with "" having the selected class, it wouldn't be sync'ed to the internal state. This meant that selecting a different option via select.value="blah" wouldn't remove the "selected" classes and attributes on the "" option, meaning it would still act like "" is selected even though selected text shows up as "blah". (closes #5646)
- `foundation.init` is no longer called twice: once by base component.ts and once by select component.ts
- `foundation.init` no longer calls `notchOutline` 3 (???) times (closes #5686)
- `foundation.init` no longer emits change event (closes #5783).

PiperOrigin-RevId: 308892334
copybara-service bot pushed a commit that referenced this issue Apr 30, 2020
…to options

- Method `layout` and private method `updateLabel` have been collapsed into one `layout` method. The reasoning is partially to reduce redundancy, but also since there's no reason they should be updated separately. The old layout method may have simply been broken since it doesn't update the label, just the notch, but lack of usage meant this problem didn't surface.
- `layoutOptions` method can now be called by the client whenever the underlying list elements change (i.e. dynamic server fetch), to ensure that the foundation's state is sync'ed up. When typeahead lands in list foundation, this will also allow an optional call to list's layout method to reinitialize typeahead cache. (closes #5646)
- The empty "" option is now properly accounted for as an option. Previously it was ignored, so if you initialized the select with "" having the selected class, it wouldn't be sync'ed to the internal state. This meant that selecting a different option via select.value="blah" wouldn't remove the "selected" classes and attributes on the "" option, meaning it would still act like "" is selected even though selected text shows up as "blah". (closes #5646)
- `foundation.init` is no longer called twice: once by base component.ts and once by select component.ts
- `foundation.init` no longer calls `notchOutline` 3 (???) times (closes #5686)
- `foundation.init` no longer emits change event (closes #5783).

PiperOrigin-RevId: 308892334
copybara-service bot pushed a commit that referenced this issue Apr 30, 2020
…to options

- Method `layout` and private method `updateLabel` have been collapsed into one `layout` method. The reasoning is partially to reduce redundancy, but also since there's no reason they should be updated separately. The old layout method may have simply been broken since it doesn't update the label, just the notch, but lack of usage meant this problem didn't surface.
- `layoutOptions` method can now be called by the client whenever the underlying list elements change (i.e. dynamic server fetch), to ensure that the foundation's state is sync'ed up. When typeahead lands in list foundation, this will also allow an optional call to list's layout method to reinitialize typeahead cache. (closes #5646)
- The empty "" option is now properly accounted for as an option. Previously it was ignored, so if you initialized the select with "" having the selected class, it wouldn't be sync'ed to the internal state. This meant that selecting a different option via select.value="blah" wouldn't remove the "selected" classes and attributes on the "" option, meaning it would still act like "" is selected even though selected text shows up as "blah". (closes #5646)
- `foundation.init` is no longer called twice: once by base component.ts and once by select component.ts
- `foundation.init` no longer calls `notchOutline` 3 (???) times (closes #5686)
- `foundation.init` no longer emits change event (closes #5783).

PiperOrigin-RevId: 308892334
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
2 participants