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

Create commonForm.js and refactor importBrowserDataPanel.js #7995

Merged
merged 2 commits into from
Apr 5, 2017
Merged

Create commonForm.js and refactor importBrowserDataPanel.js #7995

merged 2 commits into from
Apr 5, 2017

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Mar 31, 2017

Closes #7990
Closes #7996
Addresses #7989

  • Created commonForm.js to make it possible to apply commonForm to other form elements, which are .importBrowserDataPanel, .manageAutofillDataPanel, and .checkDefaultBrowserDialog for now. See: fc9902c
    It includes CommonForm, CommonFormClickable, CommonFormSection, CommonFormTitle, CommonFormSubSection, CommonFormButtonWrapper, and CommonFormBottomWrapper.

  • Created CommonFormDropdown

  • Set margin-bottom with dropdownWrapper

  • Introduced new variables commonFormBottomWrapperBackground and commonFormBackground to global.js
    The former was set to tabsBackground, and the latter to toolbarBackground.

Auditors:

Test Plan:

  1. Select "Import Browser Data..." from the main menu
  2. Make sure the dropdown works
  3. Make sure every data can be imported with the dialog
  • 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).

@luixxiul
Copy link
Contributor Author

luixxiul commented Mar 31, 2017

Here's the comparison.

screenshot 2017-03-31 12 46 03

screenshot 2017-04-01 4 10 00

@luixxiul luixxiul changed the title Create commonForm.js and refactor importaBrowserDataPanel.js Create commonForm.js and refactor importBrowserDataPanel.js Mar 31, 2017
@luixxiul luixxiul added this to the 0.14.1 milestone Mar 31, 2017
@@ -30,6 +31,12 @@ class FormDropdown extends ImmutableComponent {
}
}

class CommonFormDropdown extends ImmutableComponent {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move this component into commonForm.js as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suguru Hirahara added 2 commits April 5, 2017 03:37
Closes #7990
Closes #7996
Addresses #7989

- Created commonForm.js to make it possible to apply commonForm to other form elements, which are .importBrowserDataPanel, .manageAutofillDataPanel, and .checkDefaultBrowserDialog for now
  It includes CommonForm, CommonFormClickable, CommonFormSection, CommonFormTitle, CommonFormSubSection, CommonFormButtonWrapper, and CommonFormBottomWrapper.

- Created CommonFormDropdown
- Set margin-bottom with dropdownWrapper
- Introduced new variables commonFormBottomWrapperBackground and commonFormBackground to global.js

Auditors:

Test Plan:
1. Select "Import Browser Data..." from the main menu
2. Make sure the dropdown works
3. Make sure every data can be imported with the dialog
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.

++

@luixxiul luixxiul merged commit 2e5220c into brave:master Apr 5, 2017
@luixxiul luixxiul removed the request for review from cezaraugusto April 5, 2017 13:01
@luixxiul luixxiul deleted the commonForm-refactoring branch April 8, 2017 08:39
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.

3 participants