Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Address Nested ConstraintLayouts in search & tabs tray experiences #15788

Closed
mcomella opened this issue Oct 8, 2020 · 3 comments
Closed

Address Nested ConstraintLayouts in search & tabs tray experiences #15788

mcomella opened this issue Oct 8, 2020 · 3 comments
Assignees
Labels
eng:health Improve code health eng:qa:verified QA Verified performance Possible performance wins

Comments

@mcomella
Copy link
Contributor

mcomella commented Oct 8, 2020

Blocked by #15280 & #15644. They are addressed.

20ms tabs tray improvement.

Please remove the nested ConstraintLayouts in the search screen and tabs tray. From a correctness and performance perspective, ConstraintLayouts are designed to minimize nesting which improves perf and reduces battery use. Ideally, ConstraintLayout is the root element and contains only leaf nodes.

We added a custom lint rule #15280 to check for nested ConstraintLayouts. In order to work on this bug:

  • Delete the app/lint-baseline.xml file (it only contains these violations)
  • Run ./gradlew lintDebug and check the violation output
  • Fix the associated errors; be careful about how accessibility may change
  • (when completed) Create a new baseline file with any remaining violations

WARNING: replacing a ConstraintLayout with several nested ViewGroups is unlikely to improve performance. Remember, a goal is to reduce nesting. If you're unable to remove the extra ConstraintLayout(s) without doing this, it's probably better to suppress this violation.

Please ask the performance team if you have questions!

┆Issue is synchronized with this Jira Task

@mcomella mcomella added the performance Possible performance wins label Oct 8, 2020
@mcomella mcomella added this to Needs prioritization in Performance, front-end roadmap via automation Oct 8, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Oct 8, 2020
@mcomella mcomella added the eng:health Improve code health label Oct 21, 2020
@liuche liuche removed the needs:triage Issue needs triage label Oct 21, 2020
@liuche liuche added this to Inbox in Engineering triage via automation Oct 21, 2020
@liuche liuche moved this from Inbox to Skittle Staging Backlog in Engineering triage Oct 22, 2020
@mcomella mcomella moved this from Needs prioritization to Top 10 Inter-Team bugs in Performance, front-end roadmap Oct 28, 2020
@liuche
Copy link
Contributor

liuche commented Nov 5, 2020

Two screens involved. Can also run some performance tests (ask the perf team) afterwards.

@liuche liuche removed this from Skittle Staging Backlog in Engineering triage Nov 5, 2020
@mcarare mcarare self-assigned this Nov 17, 2020
@mcarare mcarare added this to QA Review in Hershey's 🍫 Nov 18, 2020
@mcarare mcarare moved this from QA Review to In Dev Review in Hershey's 🍫 Nov 18, 2020
@mcarare mcarare added the eng:qa:needed QA Needed label Nov 19, 2020
@mcarare
Copy link
Contributor

mcarare commented Nov 19, 2020

For QA: There is no specific testing needed, just check that the layout changes did not introduce any regressions in behavior. Thank you!

mcarare added a commit to mcarare/fenix that referenced this issue Nov 19, 2020
@sflorean
Copy link
Contributor

Tested on latest Nightly 11/19 and we didn't encounter any new issue. Devices:

  • Pixel 3 (Android 11)
  • LG G7 (Android 8.0).

Performance, front-end roadmap automation moved this from Top 10 Inter-Team bugs to Done Nov 19, 2020
Hershey's 🍫 automation moved this from In Dev Review to QA Review Nov 19, 2020
@sflorean sflorean added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Nov 19, 2020
@sflorean sflorean moved this from QA Review to Done in Hershey's 🍫 Jan 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:health Improve code health eng:qa:verified QA Verified performance Possible performance wins
Projects
Development

No branches or pull requests

4 participants