Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

navigationBar refactor tracking issue #9283

Closed
luixxiul opened this issue Jun 6, 2017 · 3 comments
Closed

navigationBar refactor tracking issue #9283

luixxiul opened this issue Jun 6, 2017 · 3 comments

Comments

@luixxiul
Copy link
Contributor

luixxiul commented Jun 6, 2017

Describe the issue you encountered: Refactor navigationBar.less with Aphrodite (tracking ticket).

navigationBar.less is obviously one of the most largest LESS files after forms.less has been almost refactored.

So I would like to propose that we should work on that file step by step, splitting the tasks, confirming each of them works nicely, rather than refactoring everything with one commit and PR.

  • Extra QA steps:
    1.
    2.
    3.

  • Any related issues:

@luixxiul
Copy link
Contributor Author

luixxiul commented Jun 7, 2017

I created a branch navigationBar-aphrodite for tasks. I'm going to split commits as clear and small as possible.

luixxiul pushed a commit that referenced this issue Jul 19, 2017
Closes #9846
Addresses #9283

Full changelog:
0d5d4fd
ab59b14
luixxiul pushed a commit that referenced this issue Jul 19, 2017
luixxiul pushed a commit that referenced this issue Jul 19, 2017
Addresses #9283

- Replace stopButton, homeButton, publisherButton, and braveMenuButton
luixxiul pushed a commit that referenced this issue Jul 19, 2017
Closes #9846
Addresses #9283

Full changelog:
0d5d4fd
ab59b14
luixxiul pushed a commit that referenced this issue Jul 19, 2017
@luixxiul luixxiul removed this from the Backlog milestone Jul 19, 2017
luixxiul pushed a commit that referenced this issue Jul 19, 2017
Addresses #9283

.navbarMenubarBlockContainer (introduced with f2426f1#diff-303f0b6a297506f2cc18bf7b4cb370c5R789, which no longer exists on the master branch.)

.inputbar-wrapper (introduced with 0e57690#diff-02c4b23ad267fe636760179e32fa29ceR141 with no clear reason. Since .inputbar-wrapper has not been used, it can be removed safely.)
luixxiul pushed a commit that referenced this issue Jul 19, 2017
luixxiul pushed a commit that referenced this issue Jul 19, 2017
Addresses #9283

Also:
- Replace stopButton, homeButton, publisherButton, and braveMenuButton
luixxiul pushed a commit that referenced this issue Jul 19, 2017
luixxiul pushed a commit that referenced this issue Jul 19, 2017
NejcZdovc pushed a commit that referenced this issue Jul 19, 2017
Addresses #9283

.navbarMenubarBlockContainer (introduced with f2426f1#diff-303f0b6a297506f2cc18bf7b4cb370c5R789, which no longer exists on the master branch.)

.inputbar-wrapper (introduced with 0e57690#diff-02c4b23ad267fe636760179e32fa29ceR141 with no clear reason. Since .inputbar-wrapper has not been used, it can be removed safely.)
NejcZdovc pushed a commit that referenced this issue Jul 19, 2017
NejcZdovc pushed a commit that referenced this issue Jul 19, 2017
Addresses #9283

Also:
- Replace stopButton, homeButton, publisherButton, and braveMenuButton
NejcZdovc pushed a commit that referenced this issue Jul 19, 2017
NejcZdovc pushed a commit that referenced this issue Jul 19, 2017
NejcZdovc pushed a commit that referenced this issue Jul 20, 2017
NejcZdovc pushed a commit that referenced this issue Jul 20, 2017
Addresses #9283

.navbarMenubarBlockContainer (introduced with f2426f1#diff-303f0b6a297506f2cc18bf7b4cb370c5R789, which no longer exists on the master branch.)

.inputbar-wrapper (introduced with 0e57690#diff-02c4b23ad267fe636760179e32fa29ceR141 with no clear reason. Since .inputbar-wrapper has not been used, it can be removed safely.)
luixxiul pushed a commit that referenced this issue Jul 23, 2017
Addresses #9283

See discussion on #9851 (comment)
luixxiul pushed a commit that referenced this issue Jul 28, 2017
Addresses #9283

.navbarMenubarBlockContainer (introduced with f2426f1#diff-303f0b6a297506f2cc18bf7b4cb370c5R789, which no longer exists on the master branch.)

.inputbar-wrapper (introduced with 0e57690#diff-02c4b23ad267fe636760179e32fa29ceR141 with no clear reason. Since .inputbar-wrapper has not been used, it can be removed safely.)
luixxiul pushed a commit that referenced this issue Jul 28, 2017
luixxiul pushed a commit that referenced this issue Jul 28, 2017
Addresses #9283

Also:
- Replace stopButton, homeButton, publisherButton, and braveMenuButton
luixxiul pushed a commit that referenced this issue Jul 28, 2017
luixxiul pushed a commit that referenced this issue Jul 28, 2017
luixxiul pushed a commit that referenced this issue Jul 28, 2017
luixxiul pushed a commit that referenced this issue Jul 28, 2017
Addresses #9283

See discussion on #9851 (comment)
NejcZdovc pushed a commit that referenced this issue Aug 8, 2017
Addresses #9283

.navbarMenubarBlockContainer (introduced with f2426f1#diff-303f0b6a297506f2cc18bf7b4cb370c5R789, which no longer exists on the master branch.)

.inputbar-wrapper (introduced with 0e57690#diff-02c4b23ad267fe636760179e32fa29ceR141 with no clear reason. Since .inputbar-wrapper has not been used, it can be removed safely.)
NejcZdovc pushed a commit that referenced this issue Aug 8, 2017
NejcZdovc pushed a commit that referenced this issue Aug 8, 2017
Addresses #9283

Also:
- Replace stopButton, homeButton, publisherButton, and braveMenuButton
NejcZdovc pushed a commit that referenced this issue Aug 8, 2017
NejcZdovc pushed a commit that referenced this issue Aug 8, 2017
NejcZdovc added a commit that referenced this issue Aug 8, 2017
NejcZdovc pushed a commit that referenced this issue Aug 8, 2017
NejcZdovc pushed a commit that referenced this issue Aug 8, 2017
@luixxiul luixxiul removed their assignment Aug 9, 2017
@cezaraugusto
Copy link
Contributor

cezaraugusto commented Aug 13, 2017

adding perf too as it can benefit several perf bottlenecks (more below).

Re-assigning @luixxiul, co-assigning me. @bsclifton and @NejcZdovc feel free to join too but will eventually be tagged for reviews. This is important for the following reasons:

With this we can:

  • improve performance removing several unused styles (this can be audited going to devtools->more tools->coverage). With Aphrodite styles are bound to the component so only rendered as needed;
  • With less styles, less work for the browser parser, less reflows/repaints, better perf. Recalling CSS is render blocking and the render tree is not built without full parse;
  • Without bloated styles, ease the task of making a critical rendering path for faster app start-up;
  • Better future unit testing with component split;
  • better DX with each component being split;
  • Big win for Aphrodite task, being this our biggest component.

Plus:

  • allow dark mode;
  • free the road to add future user-theming;

I'm changing the description to navigationBar refactor tracking issue. This also has no milestone but ideally all in 0.21.x to avoid regressions.

Recalling that most of this work was already done by @luixxiul's prior work in #9851. This is just a matter of coordinating the best takes and applying as needed.

@cezaraugusto cezaraugusto changed the title Refactor navigationBar.less with Aphrodite (tracking ticket) navigationBar refactor tracking issue Aug 13, 2017
@luixxiul
Copy link
Contributor Author

luixxiul commented Aug 13, 2017

I'm happy as long as proposed changes from me are attributed some way.

This time I'll audit if styles are successfully converted, creating no visual regressions.

@luixxiul luixxiul removed their assignment Oct 30, 2017
@bsclifton bsclifton added this to the Triage Backlog milestone Nov 27, 2017
@bsclifton bsclifton removed this from the Triage Backlog milestone May 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants