-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature/persistant columns #75
Changes from all commits
b755bce
13a90d0
21ee35b
61f7cbf
f3b21ab
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 |
---|---|---|
|
@@ -10,6 +10,7 @@ import { | |
} from "./actions"; | ||
import { AnnotationName, TOP_LEVEL_FILE_ANNOTATIONS } from "../../constants"; | ||
import AnnotationService from "../../services/AnnotationService"; | ||
import Annotation from "../../entity/Annotation"; | ||
|
||
/** | ||
* Interceptor responsible for turning REQUEST_ANNOTATIONS action into a network call for available annotations. Outputs | ||
|
@@ -20,14 +21,25 @@ const requestAnnotations = createLogic({ | |
const { getState, httpClient } = deps; | ||
const applicationVersion = interaction.selectors.getApplicationVersion(getState()); | ||
const baseUrl = interaction.selectors.getFileExplorerServiceBaseUrl(getState()); | ||
const displayAnnotations = selection.selectors.getAnnotationsToDisplay(getState()); | ||
const annotationService = new AnnotationService({ | ||
applicationVersion, | ||
baseUrl, | ||
httpClient, | ||
}); | ||
|
||
let annotations: Annotation[] = []; | ||
|
||
try { | ||
const annotations = await annotationService.fetchAnnotations(); | ||
annotations = await annotationService.fetchAnnotations(); | ||
dispatch(receiveAnnotations(annotations)); | ||
} catch (err) { | ||
console.error("Failed to fetch annotations", err); | ||
done(); | ||
return; | ||
} | ||
|
||
if (!displayAnnotations.length) { | ||
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. When I initially loaded this branch locally, got an error on load that caused the app to fail because |
||
const defaultDisplayAnnotations = compact([ | ||
find( | ||
TOP_LEVEL_FILE_ANNOTATIONS, | ||
|
@@ -40,14 +52,9 @@ const requestAnnotations = createLogic({ | |
(annotation) => annotation.name === AnnotationName.FILE_SIZE | ||
), | ||
]); | ||
|
||
dispatch(receiveAnnotations(annotations)); | ||
dispatch(selection.actions.selectDisplayAnnotation(defaultDisplayAnnotations, true)); | ||
} catch (err) { | ||
console.error("Failed to fetch annotations", err); | ||
} finally { | ||
done(); | ||
} | ||
done(); | ||
}, | ||
type: REQUEST_ANNOTATIONS, | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -157,15 +157,19 @@ export const DESELECT_DISPLAY_ANNOTATION = makeConstant( | |
); | ||
|
||
export interface DeselectDisplayAnnotationAction { | ||
payload: Annotation | Annotation[]; | ||
payload: { | ||
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. ? why 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 thought this made it a little easer to read mirroring the SelectDisplayAnnotationAction so that both have action.payload.annotation as the annotation rather than action.payload for one and action.payload.annotation for another |
||
annotation: Annotation | Annotation[]; | ||
}; | ||
type: string; | ||
} | ||
|
||
export function deselectDisplayAnnotation( | ||
annotation: Annotation | Annotation[] | ||
): DeselectDisplayAnnotationAction { | ||
return { | ||
payload: annotation, | ||
payload: { | ||
annotation, | ||
}, | ||
type: DESELECT_DISPLAY_ANNOTATION, | ||
}; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { makeReducer } from "@aics/redux-utils"; | ||
import { castArray, difference, omit, without } from "lodash"; | ||
import { castArray, difference, omit } from "lodash"; | ||
|
||
import interaction from "../interaction"; | ||
import { AnnotationName, PAST_YEAR_FILTER } from "../../constants"; | ||
|
@@ -123,9 +123,9 @@ export default makeReducer<SelectionStateBranch>( | |
sortColumn: action.payload, | ||
}), | ||
[DESELECT_DISPLAY_ANNOTATION]: (state, action) => { | ||
const displayAnnotations = without( | ||
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. Isn't this what 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 was the source of the reselect bug I was talking about, for some reason the annotations returned by the persist are slightly different and this dont get removed with the 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. Looks like lodash's |
||
state.displayAnnotations, | ||
...castArray(action.payload) | ||
// remove deselected annotation from state.displayAnnotations | ||
const displayAnnotations = state.displayAnnotations.filter( | ||
(annotation) => annotation.name !== action.payload.annotation.name | ||
); | ||
|
||
const columnWidthsToPrune = difference( | ||
|
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.
This import belongs with the above grouping (so as a new line above line 6)
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.
Moving this import causes the app to go blank. I am completely lost on why.
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.
Pulled locally and tried this as well, also caused a load error for me when put anywhere from line 8 up:
Uncaught ReferenceError: Cannot access 'Annotation' before initialization