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

Cleanup of LoginRequired #9493

Merged
merged 1 commit into from
Jun 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 24 additions & 27 deletions app/renderer/components/main/loginRequired.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,19 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const React = require('react')
const PropTypes = require('prop-types')
const {StyleSheet, css} = require('aphrodite/no-important')
const urlResolve = require('url').resolve

// Components
const Dialog = require('../common/dialog')
const Button = require('../common/button')
const {
CommonForm,
CommonFormSection,
CommonFormTitle,
CommonFormTextbox,
CommonFormButtonWrapper,
commonFormStyles
} = require('../common/commonForm')

// Actions
const appActions = require('../../../../js/actions/appActions')
Expand All @@ -20,15 +26,6 @@ const KeyCodes = require('../../../common/constants/keyCodes')
// Styles
const commonStyles = require('../styles/commonStyles')

const {
CommonForm,
CommonFormSection,
CommonFormTitle,
CommonFormTextbox,
CommonFormButtonWrapper,
commonFormStyles
} = require('../common/commonForm')

class LoginRequired extends React.Component {
constructor (props) {
super(props)
Expand All @@ -40,20 +37,18 @@ class LoginRequired extends React.Component {
this.onPasswordChange = this.onPasswordChange.bind(this)
this.onKeyDown = this.onKeyDown.bind(this)
this.onClose = this.onClose.bind(this)
this.onSave = this.onSave.bind(this)
}

focus () {
this.loginUsername.select()
this.loginUsername.focus()
}

componentDidMount () {
this.focus()
}
get detail () {
return this.props.loginRequiredDetail
}
get tabId () {
return this.props.tabId
}

onKeyDown (e) {
switch (e.keyCode) {
case KeyCodes.ENTER:
Expand All @@ -64,33 +59,39 @@ class LoginRequired extends React.Component {
break
}
}

onClose () {
appActions.setLoginResponseDetail(this.tabId)
appActions.setLoginResponseDetail(this.props.tabId)
}

onClick (e) {
e.stopPropagation()
}

onUsernameChange (e) {
this.setState({
username: e.target.value
})
}

onPasswordChange (e) {
this.setState({
password: e.target.value
})
}

onSave () {
this.focus()
this.setState({
username: '',
password: ''
})
appActions.setLoginResponseDetail(this.tabId, this.state)
appActions.setLoginResponseDetail(this.props.tabId, this.state)
}

render () {
const l10nArgs = {
host: urlResolve(this.detail.getIn(['request', 'url']), '/')
host: this.props.loginRequiredUrl
}
return <Dialog onHide={this.onClose} isClickDismiss>
<CommonForm onClick={this.onClick.bind(this)}>
Expand All @@ -106,8 +107,7 @@ class LoginRequired extends React.Component {
<label className={css(commonFormStyles.input__bottomRow)} data-l10n-id='basicAuthPasswordLabel' htmlFor='loginPassword' />
</div>
{
!this.isFolder
? <div id='loginInput' className={css(
<div id='loginInput' className={css(
commonFormStyles.inputWrapper,
commonFormStyles.inputWrapper__input
)}>
Expand All @@ -133,26 +133,23 @@ class LoginRequired extends React.Component {
/>
</div>
</div>
: null
}
</div>
</CommonFormSection>
<CommonFormButtonWrapper>
<Button l10nId='cancel' className='whiteButton' onClick={this.onClose} />
<Button l10nId='ok' className='primaryButton' onClick={this.onSave.bind(this)} />
<Button l10nId='ok' className='primaryButton' onClick={this.onSave} />
</CommonFormButtonWrapper>
</CommonForm>
</Dialog>
}
}

LoginRequired.propTypes = { frameProps: PropTypes.object }
module.exports = LoginRequired
Copy link
Member

Choose a reason for hiding this comment

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

nit: I prefer this at the end of a file

Copy link
Contributor Author

@NejcZdovc NejcZdovc Jun 23, 2017

Choose a reason for hiding this comment

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

In all components we have this like that, so I would leave it for now and we can move it in all components to the bottom in a different PR

Copy link
Member

Choose a reason for hiding this comment

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

ok I'm not concerned either way, was just noting.

Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed it as well and was trying to re-move the change.

Copy link
Contributor

Choose a reason for hiding this comment

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

@NejcZdovc please document the rule on the wiki page for refactoring work, thanks. CC @cezaraugusto


const styles = StyleSheet.create({
sectionWrapper: {
display: 'flex',
justifyContent: 'space-between'
}
})

module.exports = LoginRequired
15 changes: 12 additions & 3 deletions app/renderer/components/main/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const React = require('react')
const ImmutableComponent = require('../immutableComponent')
const Immutable = require('immutable')
const electron = require('electron')
const urlResolve = require('url').resolve
const ipc = electron.ipcRenderer

// Actions
Expand Down Expand Up @@ -614,7 +615,12 @@ class Main extends ImmutableComponent {
const releaseNotesIsVisible = this.props.windowState.getIn(['ui', 'releaseNotes', 'isVisible'])
const checkDefaultBrowserDialogIsVisible =
isFocused() && defaultBrowserState.shouldDisplayDialog(this.props.appState)
const loginRequiredDetail = activeFrame ? basicAuthState.getLoginRequiredDetail(this.props.appState, activeFrame.get('tabId')) : null
const loginRequiredDetails = activeFrame
? basicAuthState.getLoginRequiredDetail(this.props.appState, activeFrame.get('tabId'))
: null
const loginRequiredUrl = loginRequiredDetails
? urlResolve(loginRequiredDetails.getIn(['request', 'url']), '/')
: null
const customTitlebar = this.customTitlebar
const contextMenuDetail = this.props.windowState.get('contextMenuDetail')
const shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(this.props.appState, this.props.windowState, activeFrame, isFocused())
Expand Down Expand Up @@ -680,8 +686,11 @@ class Main extends ImmutableComponent {
: null
}
{
loginRequiredDetail
? <LoginRequired loginRequiredDetail={loginRequiredDetail} tabId={activeFrame.get('tabId')} />
loginRequiredUrl
? <LoginRequired
loginRequiredUrl={loginRequiredUrl}
tabId={activeFrame.get('tabId')}
/>
: null
}
{
Expand Down