Added Table View #163
Added Table View #163
Conversation
|
[diff-counting] Significant lines: 312. |
|
Visit the preview URL for this PR (updated for commit fc2ad6f): https://zing-lsc--pr163-alternate-table-view-n75p410i.web.app (expires Fri, 12 May 2023 16:07:21 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 1fc18307b178c916e2663810a6932f60b173c01b |
There was a problem hiding this comment.
Hi Jessica, thank you for getting this in so quickly! The table looks pretty nice!
I'm not approving this for now because there are some changes that I think are necessary/need to clarify with LSC. Also, the mockup in the task shows highlighting on the text with how many new students similar to dashboard; I think it's worth also implementing that.
Otherwise, great work!
There was a problem hiding this comment.
Good job Jessica! The table looks great! Thanks for working hard to incorporate the feedback :)
There was a slight bug with reloading the page and the icons being disabled, but I put a fix in the comments for each line that should be changed. I think the issue was the lack of usage of the actual state and reliance on the default and what's in history. Other than that, this should be good to go.
Make sure to test that it works again after the changes! Thanks again, Jessica :)
| <IconButton | ||
| onClick={handleClickTable} | ||
| disabled={ | ||
| state?.tableView || state?.tableView === undefined ? false : true |
There was a problem hiding this comment.
change this to state?.tableView !== undefined ? !state.tableView : !tableView
| } | ||
| const [sortedOrder, setSortedOrder] = useState<SortOrder>(() => | ||
| state?.sortedOrder ? state.sortedOrder : defaultSortingOrder | ||
| ) | ||
| const [filteredOption, setFilteredOption] = useState<FilterOption>(() => | ||
| state?.filterOption ? state.filterOption : defaultFilterOption | ||
| ) | ||
| const [tableView, setTableView] = useState(() => | ||
| state?.tableView === undefined || state?.tableView ? true : defaultView |
There was a problem hiding this comment.
change this line to state?.tableView === undefined ? defaultView : state?.tableView
| @@ -160,6 +168,7 @@ export const Dashboard = () => { | |||
| state: { | |||
| sortedOrder: event.target.value as SortOrder, | |||
| filterOption: state?.filterOption ? state.filterOption : 'no-filter', | |||
| tableView: state?.tableView ? state.tableView : defaultView, | |||
There was a problem hiding this comment.
change this line to tableView: state?.tableView !== undefined ? state.tableView : defaultView,
| //helper function to change view | ||
| const handleClickTable = () => { | ||
| setTableView( | ||
| state?.tableView === undefined || state?.tableView ? false : !defaultView |
There was a problem hiding this comment.
change this line to !tableView
| @@ -171,6 +180,28 @@ export const Dashboard = () => { | |||
| ? state.sortedOrder | |||
| : 'newest-requests-first', | |||
| filterOption: event.target.value as FilterOption, | |||
| tableView: state?.tableView ? state.tableView : defaultView, | |||
There was a problem hiding this comment.
tableView: state?.tableView !== undefined ? state.tableView : defaultView,
| state?.tableView === undefined || state?.tableView | ||
| ? false | ||
| : !defaultView, |
There was a problem hiding this comment.
!tableView (these are just toggles)
| <IconButton | ||
| onClick={handleClickTable} | ||
| disabled={ | ||
| state?.tableView || state?.tableView === undefined ? true : false |
There was a problem hiding this comment.
Change to state?.tableView !== undefined ? state.tableView : tableView
| </IconButton> | ||
| </Box> | ||
|
|
||
| {(state?.tableView ? state.tableView : defaultView) ? ( |
There was a problem hiding this comment.
state?.tableView !== undefined ? state.tableView : tableView
Summary
Test Plan
tableView.mov
Notes