Conversation
|
Please squash commits before merge :) |
efeichen
left a comment
efeichen
left a comment
There was a problem hiding this comment.
I kinda gave up at the end cuz it's a lot lol but just minor nitpicks and some formatting stuff.
Not sure ESLint can automatically fix some of these issues. If not, maybe considering Prettier? I personally like it, but feels scary dropping it in a big codebase like this so your call. Thanks Panda!
| UserYearOptions, | ||
| UserGenderOptions, | ||
| UserPronounOptions, | ||
| UserShirtSizeOptions } from '@Shared/UserEnums'; |
There was a problem hiding this comment.
Can these be aligned with line 1? Does ESLint fix these kind of formatting issues?
Since this is a refactoring PR I'm gonna be ocd about these kind of stuff 😛
|
|
||
| /** | ||
| * Create a new admin in the database and update the frontend to reflect the change | ||
| * @param {NewAdminModalFormData} newAdmin the new admin to be added to the system |
There was a problem hiding this comment.
Should there be some kind of separator (e.g. - or tab or 2 spaces) between the param name and the description? Think that'll make it easier to read. Also capitalize first char of sentence (or not) for consistency.
| * Show the alerts currently in state. | ||
| * @param {Boolean} container Determines whether the alert is wrapped in a container. | ||
| * @param {className} Override the alert with a different className. | ||
| * @returns {Component} |
There was a problem hiding this comment.
Should there be comment on return too?
| import { TESCUser } from '@Shared/ModelTypes'; | ||
| import { UserStatus } from '@Shared/UserStatus'; | ||
| import React from 'react'; | ||
| import { connect } from 'react-redux'; | ||
| import { showLoading, hideLoading } from 'react-redux-loading-bar'; | ||
| import { RouteComponentProps } from 'react-router'; | ||
| import { bindActionCreators } from 'redux'; | ||
| import { ApplicationDispatch, loadAllAdminEvents } from '~/actions'; | ||
| import { loadAllSponsorUsers } from '~/data/AdminApi'; | ||
| import { ApplicationState } from '~/reducers'; | ||
| import { applyResumeFilter } from '~/static/Resumes'; | ||
|
|
||
| import { replaceApplicants, replaceFiltered } from './actions'; | ||
| import ResumeList from './components/ResumeList'; | ||
|
|
There was a problem hiding this comment.
Why a space before the import?
lisalluo
left a comment
lisalluo
left a comment
There was a problem hiding this comment.
this is amazing stuff!!!!!!! i went through all the files and didn't see any real code changes except for an extra (accidental?) call for exporting emails
| }; | ||
|
|
||
| /** | ||
| * Render the T-Shirt size selection. |
There was a problem hiding this comment.
this comment is on the wrong thing
| import ResumeList from './components/ResumeList'; | ||
|
|
||
| type RouteProps = RouteComponentProps<{ | ||
| import { TESCUser } from '@Shared/ModelTypes'; |
There was a problem hiding this comment.
while we're looking at this file, seems like TESCUser and UserStatus aren't used anywhere in this file
| */ | ||
| exportEmails = () => { | ||
| const eventAlias = this.props.event.alias; | ||
| exportUsers(eventAlias, true) |
There was a problem hiding this comment.
there are two calls to exportUsers() here
|
|
||
| /** | ||
| * Create the input field for aqpplicant GPAs. | ||
| * @param info redux-form object of the institution the applicatnt is from |
There was a problem hiding this comment.
if we're being really nitpicky, applicant is spelled wrong twice :)
|
|
||
| /** | ||
| * Create the input field for major and year information of the applicant. | ||
| * @param info redux-form object of the institution the applicatnt is from |
There was a problem hiding this comment.
nitpicky: applicant spelling
| } | ||
| const adminToBeRendered = {...admin, | ||
| lastAccessed: admin.lastAccessed | ||
| ? moment(admin.lastAccessed).fromNow() |
There was a problem hiding this comment.
can you align these lines somehow
| if (logo) { | ||
| promiseReq.attach('logo', logo[0]) | ||
| if (logo) { | ||
| promiseReq.attach('logo', logo[0]); |
| import UsersPage from './pages/UsersPage'; | ||
| import PreviewApplication from './pages/EventPage/components/PreviewApplication'; | ||
|
|
||
| /* |
There was a problem hiding this comment.
i think these comments are a little misplaced?
| ├── HomePage | ||
| * This is the main page that loads when a user goes to www.tesc.events | ||
| * This page handles 2 states | ||
| - showing all apply-able events when the user is not logged in, and show |
3 participants
The code should not have changed at all (only comments). Please scroll through and make user not a single line of runnable code has changed (expect lint fixes :) )