External project overview landing page - #1179
Conversation
There was a problem hiding this comment.
Changes to this file should be backported to the #1178 PR since nothing new is being added (just corrections on changes made by the first PR)
| const parentProjectColumns: ColumnsType<FMSParentProject> = [ | ||
| { |
There was a problem hiding this comment.
parentProjectColumns should be in a useMemo like
const parentProjectColumns = useMemo<ColumnsType<FMSParentProject>>(() => [...], [...])
| expandedRowRender: (parentProject) => {const internalProjects = (parentProject.projects ?? []) | ||
| .map((projectID) => internalProjectsByID[projectID]) | ||
| .filter((project): project is FMSProject =>project !== undefined) |
There was a problem hiding this comment.
(project): project is FMSProject =>project !== undefinedThat's a really fascinating syntax. I was confused at first as to what it is. But now I get it.
expandedRowRender: (parentProject) => {const internalProjects = (parentProject.projects ?? [])The const internalProjects should be on another line.
Lastly,
I believe it's better to use reduce than map+filter. (javascript sadly doesn't have mapFilter or filterMap)
| placeholder="Search External project name" | ||
| value={selectedKeys[0]} | ||
| onChange={(event) => setSelectedKeys(event.target.value ? [event.target.value] : [])} | ||
| onPressEnter={() => confirm()} |
There was a problem hiding this comment.
Theres is discrepancy in the way the search behave compared to the rest of the app. Your approach is not bad but it clashes with other pages.
- Typically on pressing the search magnifying glass icon, the focus is given to the input field in order to allow direct entry instead of having to manually giving the focus to the fields first.
- Most (if not all) search in the app will perform a debounced search as the user type the search content in and a search is also performed on pressing enter before closing the modal window.
- A search on the external project ID would be good to make a quick search in case the user knows the ID.
Matching the other pages behaviour would make it more natural to the user.
There was a problem hiding this comment.
Noted, with Thanks. Working in progress ....
There was a problem hiding this comment.
Points 1,2,3 are fixed ... (using getFilterPropsForDescription)
|
This is starting to shape up interestingly. I think in the future we should move the PI to the parent project in order to be able to filter by it... This can be done later. |
| dataIndex: 'external_id', | ||
| key: 'external_id', | ||
| width: 120, | ||
| render: (externalID: string, parentProject: FMSParentProject) => (<Link to={`/external-projects-overview/${parentProject.id}#submissions`}>{externalID}</Link>) |
There was a problem hiding this comment.
This link leads to nowhere yet. Keep each PR centered around itself. Make it so the link does not appear here unless there is a working destination. If you build a PR on top of the stack of PR you can add the link to the child PR once you define the destination. We want to be able to merge each PR being confident the app is self contained without having to worry about dead links and dangling parts.
|
|
||
|
|
||
| <PageContent> | ||
| {error && (<Alert type="error" title={error} style={{ marginBottom: 16 }} />)} |
There was a problem hiding this comment.
I am not sure this would be very useful. The page would not render most likely if the backend was down. The actual information provided is very limited. I would tend to go for simplicity (removing error state) here given the small value of providing this error message to the user.
| dataIndex: 'external_id', | ||
| key: 'external_id', | ||
| width: 120, | ||
| render: (externalID: string, parentProject: FMSParentProject) => (<Link to={`/external-projects-overview/${parentProject.id}#submissions`}>{externalID}</Link>) |
There was a problem hiding this comment.
what does #submissions points to ? Looks like it is left over from an older version.
There was a problem hiding this comment.
Fixed. I removed the link because the target details page is not part of this PR.
…dd utility functions
…ass icon, the focus is given to the input field in order to allow direct entry
7d927db to
e52ea58
Compare
|
Closing this PR as it has been replaced by #1185, which consolidates the entire stack into a single PR. |
3 participants
Note
Please review #1178 first if you haven't already.
Adds a Parent Projects table displaying the list of external projects . Each external project is described with its external ID, name, and number of associated Freezeman projects. Each row can be expanded to show the corresponding internal projects and their key information. The page also supports filtering Parent Projects by external project name.