-
Notifications
You must be signed in to change notification settings - Fork 479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Manage Students Table: add multiple students button #20935
Changes from 7 commits
423d934
c16ebcd
7479a79
fca66c1
5a65ab7
535fcf2
6f8ecf5
ae72f3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
import React, {Component, PropTypes} from 'react'; | ||
import {connect} from 'react-redux'; | ||
import {addMultipleAddRows} from './manageStudentsRedux'; | ||
import Button from '../Button'; | ||
import i18n from "@cdo/locale"; | ||
import BaseDialog from '../BaseDialog'; | ||
import DialogFooter from "../teacherDashboard/DialogFooter"; | ||
|
||
const styles = { | ||
dialog: { | ||
paddingLeft: 20, | ||
paddingRight: 20, | ||
paddingBottom: 20 | ||
} | ||
}; | ||
|
||
class AddMultipleStudents extends Component { | ||
static propTypes = { | ||
// Provided by redux | ||
addMultipleStudents: PropTypes.func.isRequired, | ||
}; | ||
|
||
state = { | ||
openDialog: false, | ||
}; | ||
|
||
openDialog = () => { | ||
this.setState({openDialog: true}); | ||
}; | ||
|
||
closeDialog = () => { | ||
this.setState({openDialog: false}); | ||
}; | ||
|
||
add = () => { | ||
const value = this.refs.studentsTextBox.value; | ||
this.props.addMultipleStudents(value.split("\n")); | ||
this.closeDialog(); | ||
}; | ||
|
||
render() { | ||
return ( | ||
<div> | ||
<Button | ||
onClick={this.openDialog} | ||
color={Button.ButtonColor.gray} | ||
text={i18n.addStudentsMultiple()} | ||
/> | ||
<BaseDialog | ||
useUpdatedStyles | ||
uncloseable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This dialog has a "Cancel" button. Is there a good reason to make it uncloseable? This is worth diverging from spec to provide user-friendly behavior, unless there's a specific reason for it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope - I copied this from somewhere and forgot to remove it. |
||
isOpen={this.state.openDialog} | ||
style={styles.dialog} | ||
> | ||
<h2>{i18n.addStudentsMultiple()}</h2> | ||
<div> | ||
{i18n.addStudentsMultipleInstructions()} | ||
</div> | ||
<textarea name="textarea" rows="15" cols="70" ref="studentsTextBox"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May not necessary to fix in this PR, especially if there will be a styling pass later, but it'd be nice to size this textbox to match the dialog width instead of using rows/cols. Also not sure it should be resizable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll do a style pass all together 👍 |
||
</textarea> | ||
<DialogFooter> | ||
<Button | ||
text={i18n.dialogCancel()} | ||
onClick={this.closeDialog} | ||
color={Button.ButtonColor.gray} | ||
/> | ||
<Button | ||
text={i18n.done()} | ||
onClick={this.add} | ||
color={Button.ButtonColor.orange} | ||
/> | ||
</DialogFooter> | ||
</BaseDialog> | ||
</div> | ||
); | ||
} | ||
} | ||
|
||
export default connect(state => ({}), dispatch => ({ | ||
addMultipleStudents(names) { | ||
dispatch(addMultipleAddRows(names)); | ||
}, | ||
}))(AddMultipleStudents); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,9 +11,11 @@ import ManageStudentsNameCell from './ManageStudentsNameCell'; | |
import ManageStudentsAgeCell from './ManageStudentsAgeCell'; | ||
import ManageStudentsGenderCell from './ManageStudentsGenderCell'; | ||
import ManageStudentsActionsCell from './ManageStudentsActionsCell'; | ||
import {convertStudentDataToArray, ADD_STATUS} from './manageStudentsRedux'; | ||
import {convertStudentDataToArray, ADD_STATUS, ROW_TYPE} from './manageStudentsRedux'; | ||
import { connect } from 'react-redux'; | ||
import Notification, {NotificationType} from '../Notification'; | ||
import AddMultipleStudents from './AddMultipleStudents'; | ||
import Button from '../Button'; | ||
|
||
export const studentSectionDataPropType = PropTypes.shape({ | ||
id: PropTypes.number.isRequired, | ||
|
@@ -25,7 +27,7 @@ export const studentSectionDataPropType = PropTypes.shape({ | |
secretPicturePath: PropTypes.string, | ||
sectionId: PropTypes.number, | ||
loginType: PropTypes.string, | ||
isAddRow: PropTypes.bool, | ||
rowType: PropTypes.oneOf(Object.values(ROW_TYPE)), | ||
}); | ||
|
||
/** @enum {number} */ | ||
|
@@ -71,20 +73,25 @@ const passwordFormatter = (loginType, {rowData}) => { | |
}; | ||
|
||
// The "add row" should always be pinned to the top when sorting. | ||
// The "new student rows" should always be next. | ||
// This function takes into account having multiple "add rows" | ||
const sortRows = (data, columnIndexList, orderList) => { | ||
export const sortRows = (data, columnIndexList, orderList) => { | ||
let addRows = []; | ||
let newStudentRows = []; | ||
let studentRows = []; | ||
for (let i = 0; i<data.length; i++) { | ||
if (data[i].isAddRow) { | ||
if (data[i].rowType === ROW_TYPE.add) { | ||
addRows.push(data[i]); | ||
} else if (data[i].rowType === ROW_TYPE.newStudent) { | ||
newStudentRows.push(data[i]); | ||
} else { | ||
studentRows.push(data[i]); | ||
} | ||
} | ||
addRows = orderBy(addRows, columnIndexList, orderList); | ||
newStudentRows = orderBy(newStudentRows, columnIndexList, orderList); | ||
studentRows = orderBy(studentRows, columnIndexList, orderList); | ||
return addRows.concat(studentRows); | ||
return addRows.concat(newStudentRows).concat(studentRows); | ||
}; | ||
|
||
class ManageStudentsTable extends Component { | ||
|
@@ -151,11 +158,29 @@ class ManageStudentsTable extends Component { | |
isEditing={rowData.isEditing} | ||
isSaving={rowData.isSaving} | ||
disableSaving={disableSaving} | ||
isAddRow={rowData.isAddRow} | ||
rowType={rowData.rowType} | ||
/> | ||
); | ||
}; | ||
|
||
actionsHeaderFormatter = () => { | ||
const numberOfEditingRows = Object.keys(this.props.editingData).length; | ||
return ( | ||
<div> | ||
{numberOfEditingRows > 1 && | ||
<Button | ||
onClick={()=>{console.log("save all");}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the TODO referenced in the description. |
||
color={Button.ButtonColor.orange} | ||
text={i18n.saveAll()} | ||
/> | ||
} | ||
{numberOfEditingRows <= 1 && | ||
i18n.actions() | ||
} | ||
</div> | ||
); | ||
}; | ||
|
||
getSortingColumns = () => { | ||
return this.state.sortingColumns || {}; | ||
}; | ||
|
@@ -266,6 +291,7 @@ class ManageStudentsTable extends Component { | |
property: 'actions', | ||
header: { | ||
label: i18n.actions(), | ||
format: this.actionsHeaderFormatter, | ||
props: { | ||
style: { | ||
...tableLayoutStyles.headerCell, | ||
|
@@ -303,24 +329,29 @@ class ManageStudentsTable extends Component { | |
sort: sortRows, | ||
})(this.props.studentData); | ||
|
||
const {addStatus, loginType} = this.props; | ||
|
||
return ( | ||
<div> | ||
{this.props.addStatus === ADD_STATUS.success && | ||
{addStatus === ADD_STATUS.success && | ||
<Notification | ||
type={NotificationType.success} | ||
notice={i18n.manageStudentsNotificationSuccess()} | ||
details={i18n.manageStudentsNotificationAddSuccess()} | ||
dismissible={false} | ||
/> | ||
} | ||
{this.props.addStatus === ADD_STATUS.fail && | ||
{addStatus === ADD_STATUS.fail && | ||
<Notification | ||
type={NotificationType.failure} | ||
notice={i18n.manageStudentsNotificationFailure()} | ||
details={i18n.manageStudentsNotificationCannotAdd()} | ||
dismissible={false} | ||
/> | ||
} | ||
{(loginType === SectionLoginType.word || loginType === SectionLoginType.picture) && | ||
<AddMultipleStudents/> | ||
} | ||
<Table.Provider | ||
columns={columns} | ||
style={tableLayoutStyles.table} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super-picky nit: I'd name this something like
isDialogOpen
.openDialog
sounds like an action to me. In fact, you have a public methodopenDialog
just below.