-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Large] Create UI for metadata editing workflow #338
[Large] Create UI for metadata editing workflow #338
Conversation
|
||
import styles from "./EditMetadata.module.css"; | ||
|
||
enum EditMetadataPathway { |
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.
Maybe const enum
here
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.
is there a particular reason const enum would be needed for this one?
UX bugs from @lynwilhelm :
Global issues not specific to this ticket: |
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.
Anya! You nailed this 💯
packages/core/components/EditMetadata/ExistingAnnotationPathway.tsx
Outdated
Show resolved
Hide resolved
@@ -135,6 +136,24 @@ export default (filters?: FileFilter[], onDismiss?: () => void) => { | |||
], | |||
}, | |||
}, | |||
...(isQueryingAicsFms |
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.
Did you create a ticket for adding this capability for non-aics data sources? I think it wouldn't be very difficult to do via SQL like:
UPDATE ${datasourceName}
SET ${columnToUpdate} = ${valueToReplaceWith}
WHERE ${here either select by row number or the internally made BFF ID} = ${list of row numbers or bff ids}
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.
honestly I forgot we were planning to allow editing on non-aics data sources. Made ticket: #347
const fileSelection = useSelector(selection.selectors.getFileSelection); | ||
const fileCount = fileSelection.count(); | ||
// Don't allow users to edit top level annotations (e.g., File Name) | ||
const annotationOptions = useSelector(metadata.selectors.getSortedAnnotations) |
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.
Unsure why I have never attached JS array functions onto the selector like this... looks clean!
This PR creates the main part of the UI for editing file metadata from BFF based on the Figma designs. Does not include any backend logic.
Temporarily deployed to stagingPart of #96
Reviewing
Lots of changes with a slightly complicated file structure, so here's suggested review order:
There's also a brand new 'ComboBox' component (dropdown selector with search), which is a styled wrapper for the FluentUI combo box
To Dos (UI only)
Screenshots
Existing annotation pathway:
New annotation pathway: