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

Refactored history, bookmarks, sortableTable, and about with Aphrodite #13590

Closed
wants to merge 1 commit into from
Closed

Refactored history, bookmarks, sortableTable, and about with Aphrodite #13590

wants to merge 1 commit into from

Conversation

jasonrsadler
Copy link
Contributor

@jasonrsadler jasonrsadler commented Mar 24, 2018

Refactored history, bookmarks, sortableTable, and about with Aphrodite

Fixes and/or applies to issue #10880, issue #10898, issue #10899, issue #8056

Also applies to #8587 and eliminates the need for history.less

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

Request review @luixxiul @bsclifton
Request security review @diracdeltas

There are 'hacks' that do exist due to aphrodite being a component styler and not necessarily a 'selector styler'. Please advise on any requested changes.

If these changes are acceptable, 'history.less' can be removed. 'sortableTable.less' still has trivial links.

@jasonrsadler jasonrsadler changed the title Refactory history, bookmarks, sortableTable, and about with Aphrodite Refactored history, bookmarks, sortableTable, and about with Aphrodite Mar 24, 2018
WIP - updating history styling

WIP -- updating history styling

WIP -- updating history styles to use aphrodite

WIP -- aphrodite modifications

WIP -- updating history and sortableTable styles

WIP -- continuing style updates

WIP -- cleaning up about.js and brave.js sortableTable deps

WIP -- correcting styling with sort arrow under sortableTable

Refactory history, bookmarks, sortableTable, and about with Aphrodite

Auditors:

Test Plan:
@NejcZdovc NejcZdovc added this to the Completed work milestone May 7, 2018
@jasonrsadler
Copy link
Contributor Author

@NejcZdovc @cezaraugusto
Cleaning up open PRs

@diracdeltas
Copy link
Member

sec review passes 👍

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Thanks for putting the work for this refactoring! At this point, I think pure Aphrodite refactors might be introducing too much risk and testing time (while not delivering enough benefit)

I'd like to encourage you to check out Brave UI and hit up @cezaraugusto - I believe he's already started some work on the sortableTable. Would be great to see this work componentized and usable by other projects 😄

@bsclifton bsclifton closed this May 10, 2018
@bsclifton bsclifton removed this from the Completed work milestone May 10, 2018
@jasonrsadler
Copy link
Contributor Author

@bsclifton 100 % agree on the risk for this. There's already a ton that's changed with sortableTable since this PR which probably cancels out a lot. Either way, it allowed me to get more familiar with some of the UI aspects of the browser. Just needed to get this off my list :). Thanks.

@jasonrsadler jasonrsadler deleted the update-aphrodite-issue-#8587 branch May 10, 2018 11:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants