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

Reorganize dropdown.js #11064

Merged
merged 1 commit into from
Oct 14, 2017
Merged

Reorganize dropdown.js #11064

merged 1 commit into from
Oct 14, 2017

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Sep 21, 2017

Closes #11063

  • Replace specific components with ones styled with data attirbutions

Auditors:

Test Plan:

  1. Open about:styles
  2. Click dropdowns
  3. Make sure they are properly styled
  4. Open about:autofill
  5. Click Add Address
  6. Make sure the country dropdown has 100% width

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

Closes #11063

- Replace specific components with ones styled with data attirbutions

Auditors:

Test Plan:
1. Open about:styles
2. Click `dropdowns`
3. Make sure they are properly styled
4. Open about:autofill
5. Click `Add Address`
6. Make sure the country dropdown has 100% width
@luixxiul luixxiul added polish Nice to have — usually related to front-end/visual tasks. refactoring/aphrodite labels Sep 21, 2017
@luixxiul luixxiul added this to the 0.21.x (Nightly Channel) milestone Sep 21, 2017
@luixxiul luixxiul self-assigned this Sep 21, 2017
@codecov-io
Copy link

Codecov Report

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

@@            Coverage Diff             @@
##           master   #11064      +/-   ##
==========================================
+ Coverage   53.32%   53.33%   +<.01%     
==========================================
  Files         251      251              
  Lines       21807    21798       -9     
  Branches     3401     3401              
==========================================
- Hits        11629    11626       -3     
+ Misses      10178    10172       -6
Flag Coverage Δ
#unittest 53.33% <100%> (ø) ⬆️
Impacted Files Coverage Δ
app/renderer/components/common/commonForm.js 31.48% <ø> (+1.65%) ⬆️
app/renderer/components/common/dropdown.js 100% <ø> (+9.37%) ⬆️
...r/components/preferences/payment/enabledContent.js 94.06% <100%> (ø) ⬆️

@ghost ghost removed the priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). label Sep 26, 2017
@cezaraugusto
Copy link
Contributor

should this land before #10978 or the opposite?

@luixxiul
Copy link
Contributor Author

luixxiul commented Oct 13, 2017

I cannot remember well how much this PR is dependent on #10978 but I don't think they addresses different issues (#10978 does not modify dropdown.js) so let's merge #10978 first in case. If there would be conflicts, I'll fix those.

@luixxiul luixxiul merged commit c1e9f55 into brave:master Oct 14, 2017
@luixxiul luixxiul deleted the reorganize-dropdown branch October 14, 2017 04:20
@luixxiul luixxiul modified the milestones: 0.21.x (Developer Channel), 0.22.x (Nightly Channel) Oct 14, 2017
@bbondy bbondy modified the milestones: 0.22.x (Developer Channel), 0.23.x (Nightly Channel) Feb 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
polish Nice to have — usually related to front-end/visual tasks. refactoring/aphrodite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants