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

Commit

Permalink
Polish CommonForm
Browse files Browse the repository at this point in the history
- Replace FlyoutDialog with CommonForm to dismiss the possibility of visual regression
- Reduce components by adding props
- Add 'CommonFormSection title' to bookmark hanger
- Update to the modified BEM style
- Fix data-test-id

Auditors:

Test Plan:
1. Open about:styles
2. Click 'commonForm'
3. Make sure the common form panel is properly displayed
  • Loading branch information
Suguru Hirahara authored and cezaraugusto committed Nov 21, 2017
1 parent 32dca6f commit c5186d5
Show file tree
Hide file tree
Showing 13 changed files with 187 additions and 283 deletions.
14 changes: 6 additions & 8 deletions app/renderer/components/autofill/autofillAddressPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,9 @@ const ReduxComponent = require('../reduxComponent')
const Dialog = require('../common/dialog')
const Button = require('../common/button')
const {
CommonFormLarge,
CommonForm,
CommonFormSection,
CommonFormTitle,
CommonFormDropdown,
CommonFormButtonWrapper,
commonFormStyles
} = require('../common/commonForm')

Expand Down Expand Up @@ -160,8 +158,8 @@ class AutofillAddressPanel extends React.Component {

render () {
return <Dialog onHide={this.onHide} testId='autofillAddressPanel' isClickDismiss>
<CommonFormLarge onClick={this.onClick}>
<CommonFormTitle data-l10n-id='editAddress' />
<CommonForm large onClick={this.onClick}>
<CommonFormSection title l10nId='editAddress' />
<CommonFormSection>
<div className={css(commonFormStyles.sectionWrapper)}>
<div className={css(commonFormStyles.inputWrapper, commonFormStyles.inputWrapper__label)}>
Expand Down Expand Up @@ -273,7 +271,7 @@ class AutofillAddressPanel extends React.Component {
</div>
</div>
</CommonFormSection>
<CommonFormButtonWrapper>
<CommonFormSection buttons>
<Button className='whiteButton'
l10nId='cancel'
testId='cancelAddressButton'
Expand All @@ -285,8 +283,8 @@ class AutofillAddressPanel extends React.Component {
testId='saveAddressButton'
onClick={this.onSave}
/>
</CommonFormButtonWrapper>
</CommonFormLarge>
</CommonFormSection>
</CommonForm>
</Dialog>
}
}
Expand Down
16 changes: 7 additions & 9 deletions app/renderer/components/autofill/autofillCreditCardPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ const Button = require('../common/button')
const {
CommonForm,
CommonFormSection,
CommonFormTitle,
CommonFormDropdown,
CommonFormTextbox,
CommonFormButtonWrapper,
commonFormStyles
} = require('../common/commonForm')

Expand Down Expand Up @@ -128,9 +126,9 @@ class AutofillCreditCardPanel extends React.Component {
render () {
return <Dialog onHide={this.onHide} testId='autofillCreditCardPanel' isClickDismiss>
<CommonForm onClick={this.onClick}>
<CommonFormTitle
data-test-id='manageAutofillDataTitle'
data-l10n-id='editCreditCard'
<CommonFormSection title
l10nId='editCreditCard'
testId='manageAutofillDataTitle'
/>
<CommonFormSection>
<div className={css(commonFormStyles.sectionWrapper)}>
Expand Down Expand Up @@ -202,7 +200,7 @@ class AutofillCreditCardPanel extends React.Component {
</div>
</div>
</CommonFormSection>
<CommonFormButtonWrapper>
<CommonFormSection buttons>
<Button className='whiteButton'
l10nId='cancel'
testId='cancelCreditCardButton'
Expand All @@ -214,14 +212,12 @@ class AutofillCreditCardPanel extends React.Component {
testId='saveCreditCardButton'
onClick={this.onSave}
/>
</CommonFormButtonWrapper>
</CommonFormSection>
</CommonForm>
</Dialog>
}
}

module.exports = ReduxComponent.connect(AutofillCreditCardPanel)

const styles = StyleSheet.create({
// Copied from textbox.js
input: {
Expand All @@ -239,3 +235,5 @@ const styles = StyleSheet.create({
marginLeft: `calc(${globalStyles.spacing.dialogInsideMargin} / 3)`
}
})

module.exports = ReduxComponent.connect(AutofillCreditCardPanel)
30 changes: 7 additions & 23 deletions app/renderer/components/bookmarks/addEditBookmarkFolder.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@

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

// Components
const ReduxComponent = require('../reduxComponent')
const Dialog = require('../common/dialog')
const AddEditBookmarkFolderForm = require('./addEditBookmarkFolderForm')
const {CommonFormBookmarkHanger} = require('../common/commonForm')
const {
CommonFormHanger,
CommonFormSection
} = require('../common/commonForm')

// State
const bookmarkFoldersState = require('../../../common/state/bookmarkFoldersState')
Expand All @@ -23,9 +25,6 @@ const windowActions = require('../../../../js/actions/windowActions')
const cx = require('../../../../js/lib/classSet')
const bookmarkFoldersUtil = require('../../../common/lib/bookmarkFoldersUtil')

// Styles
const globalStyles = require('../styles/global')

class AddEditBookmarkFolder extends React.Component {
constructor (props) {
super(props)
Expand Down Expand Up @@ -68,11 +67,8 @@ class AddEditBookmarkFolder extends React.Component {
return <Dialog className={cx({
bookmarkDialog: true
})} onHide={this.onClose} isClickDismiss>
<CommonFormBookmarkHanger onClick={this.onClick}>
<div className={cx({
[css(styles.commonFormSection)]: true,
[css(styles.commonFormTitle)]: true
})} data-l10n-id={this.props.heading} />
<CommonFormHanger bookmark onClick={this.onClick}>
<CommonFormSection title l10nId={this.props.heading} />
<AddEditBookmarkFolderForm
folderName={this.props.folderName}
editKey={this.props.editKey}
Expand All @@ -83,21 +79,9 @@ class AddEditBookmarkFolder extends React.Component {
isDisabled={!this.props.isFolderNameValid}
hasBookmarks={this.props.hasBookmarks}
/>
</CommonFormBookmarkHanger>
</CommonFormHanger>
</Dialog>
}
}

const styles = StyleSheet.create({
// Copied from commonForm.js
commonFormSection: {
// PR #7985
margin: `${globalStyles.spacing.dialogInsideMargin} 30px`
},
commonFormTitle: {
color: globalStyles.color.braveOrange,
fontSize: '1.2em'
}
})

module.exports = ReduxComponent.connect(AddEditBookmarkFolder)
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const BrowserButton = require('../common/browserButton')
const {
CommonFormSection,
CommonFormDropdown,
CommonFormButtonWrapper,
commonFormStyles
} = require('../common/commonForm')

Expand Down Expand Up @@ -193,7 +192,7 @@ class AddEditBookmarkFolderForm extends React.Component {
</div>
</div>
</CommonFormSection>
<CommonFormButtonWrapper>
<CommonFormSection buttons>
{
this.props.editKey != null
? <BrowserButton groupedItem secondaryColor
Expand All @@ -213,7 +212,7 @@ class AddEditBookmarkFolderForm extends React.Component {
disabled={this.state.isDisabled}
onClick={this.onSave}
/>
</CommonFormButtonWrapper>
</CommonFormSection>
</div>
}
}
Expand Down
5 changes: 2 additions & 3 deletions app/renderer/components/bookmarks/addEditBookmarkForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const {
CommonFormSection,
CommonFormDropdown,
CommonFormTextbox,
CommonFormButtonWrapper,
commonFormStyles
} = require('../common/commonForm')

Expand Down Expand Up @@ -229,7 +228,7 @@ class AddEditBookmarkForm extends React.Component {
</div>
</div>
</CommonFormSection>
<CommonFormButtonWrapper>
<CommonFormSection buttons>
{
this.props.editKey != null
? <BrowserButton groupedItem secondaryColor
Expand All @@ -249,7 +248,7 @@ class AddEditBookmarkForm extends React.Component {
disabled={this.state.isDisabled}
onClick={this.onSave}
/>
</CommonFormButtonWrapper>
</CommonFormSection>
</div>
}
}
Expand Down
29 changes: 8 additions & 21 deletions app/renderer/components/bookmarks/addEditBookmarkHanger.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ const ReduxComponent = require('../reduxComponent')
const Dialog = require('../common/dialog')
const AddEditBookmarkForm = require('./addEditBookmarkForm')
const {
CommonFormBookmarkHanger,
CommonFormBottomWrapper
CommonFormHanger,
CommonFormSection
} = require('../common/commonForm')

// States
Expand Down Expand Up @@ -108,8 +108,8 @@ class AddEditBookmarkHanger extends React.Component {
bookmarkDialog: this.props.isModal,
bookmarkHanger: !this.props.isModal,
[css(styles.bookmarkHanger)]: !this.props.isModal
})} onHide={this.onClose} isClickDismiss>
<CommonFormBookmarkHanger onClick={this.onClick}>
})} isClickDismiss>
<CommonFormHanger bookmark onClick={this.onClick}>
{
!this.props.isModal
? <div className={cx({
Expand All @@ -118,10 +118,7 @@ class AddEditBookmarkHanger extends React.Component {
})} />
: null
}
<div className={cx({
[css(styles.commonFormSection)]: true,
[css(styles.commonFormTitle)]: true
})} data-l10n-id={this.props.heading} />
<CommonFormSection title l10nId={this.props.heading} />
<AddEditBookmarkForm
title={this.props.title}
editKey={this.props.editKey}
Expand All @@ -136,30 +133,20 @@ class AddEditBookmarkHanger extends React.Component {
/>
{
!this.props.isModal
? <CommonFormBottomWrapper>
? <CommonFormSection bottom>
<div className={css(styles.bookmark__bottomWrapper, styles.bottomWrapper__cursor)}
data-test-id='viewBookmarks'
data-l10n-id='viewBookmarks'
onClick={this.onViewBookmarks} />
</CommonFormBottomWrapper>
</CommonFormSection>
: null
}
</CommonFormBookmarkHanger>
</CommonFormHanger>
</Dialog>
}
}

const styles = StyleSheet.create({
// Copied from commonForm.js
commonFormSection: {
// PR #7985
margin: `${globalStyles.spacing.dialogInsideMargin} 30px`
},
commonFormTitle: {
color: globalStyles.color.braveOrange,
fontSize: '1.2em'
},

bookmarkHanger: {
// See: #9040
justifyContent: 'flex-start !important',
Expand Down
Loading

0 comments on commit c5186d5

Please sign in to comment.