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

Refactor findbar with Aphrodite #11417

Merged
merged 1 commit into from
Oct 13, 2017
Merged

Refactor findbar with Aphrodite #11417

merged 1 commit into from
Oct 13, 2017

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Oct 10, 2017

Closes #9233

  • Apply BrowserButton to the close button
  • Apply Textbox to the input element
  • Add color styles to theme.js

Auditors: @cezaraugusto

Test Plan:

  1. Run npm run test -- --grep='findbar'
  2. Open facebook.com
  3. Display find bar
  4. Make sure the bar is displayed properly

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.

Test Plan:

Reviewer Checklist:

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

@luixxiul
Copy link
Contributor Author

luixxiul commented Oct 10, 2017

This PR's commit was cherry-picked from 56b0f0c of #10951 to make reviewing easier.

@codecov-io
Copy link

codecov-io commented Oct 11, 2017

Codecov Report

Merging #11417 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #11417      +/-   ##
==========================================
+ Coverage   52.48%   52.49%   +<.01%     
==========================================
  Files         268      268              
  Lines       25229    25231       +2     
  Branches     4026     4026              
==========================================
+ Hits        13242    13244       +2     
  Misses      11987    11987
Flag Coverage Δ
#unittest 52.49% <100%> (ø) ⬆️
Impacted Files Coverage Δ
app/renderer/components/styles/global.js 100% <ø> (ø) ⬆️
app/renderer/components/styles/theme.js 100% <100%> (ø) ⬆️

Closes #9233

- Apply BrowserButton to the close button
- Apply Textbox to the input element
- Add color styles to theme.js

Auditors:

Test Plan:
1. Run `npm run test -- --grep='findbar'`
2. Open facebook.com
3. Display find bar
4. Make sure the bar is displayed properly
@luixxiul luixxiul requested a review from petemill October 12, 2017 05:09
@luixxiul luixxiul added this to the 0.21.x (Nightly Channel) milestone Oct 12, 2017
disabled={this.props.numberOfMatches <= 0}
onClick={this.onFindPrev}
// See autofillAddressPanel.js
// Ref: https://github.com/brave/browser-laptop/pull/7164#discussion_r100586892
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@petemill do you think this applies to this case?

Copy link
Member

Choose a reason for hiding this comment

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

@luixxiul if you are asking will the ref={ ... } work here - then yes because it's directly on html element.

@@ -4,7 +4,7 @@

const React = require('react')
const Immutable = require('immutable')
const {StyleSheet, css} = require('aphrodite')
const {StyleSheet, css} = require('aphrodite/no-important')
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for this

Copy link
Contributor

@cezaraugusto cezaraugusto 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 moving selectors to data attr

@cezaraugusto cezaraugusto merged commit 8d1952e into brave:master Oct 13, 2017
@cezaraugusto cezaraugusto deleted the refactor-findbar branch October 13, 2017 20:45
cezaraugusto added a commit that referenced this pull request Oct 13, 2017
@cezaraugusto
Copy link
Contributor

0.21.x a950f88

@luixxiul
Copy link
Contributor Author

++ thanks for moving selectors to data attr

please wait for another big update... :-)

@bbondy bbondy modified the milestones: 0.21.x (Developer Channel), 0.20.x (Beta Channel) Oct 25, 2017
@luixxiul luixxiul removed the request for review from petemill November 1, 2017 20:08
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.

Refactor findbar with Aphrodite
5 participants