Adding type to layer dialog hooks #937
Conversation
|
|
|
Integration tests report: appsharing.space |
packages/base/src/dialogs/symbology/components/color_ramp/ColorRamp.tsx
Outdated
Show resolved
Hide resolved
…in SingleBandPseudoColor, Categorized, and Graduated components
…SingleBandPseudoColor and Graduated components
mfisher87
left a comment
mfisher87
left a comment
There was a problem hiding this comment.
I have to leave for a conference soon, so just leaving some thoughts for now :) I'd like to help out a bit more soon!
packages/base/src/dialogs/symbology/components/color_ramp/ModeSelectRow.tsx
Outdated
Show resolved
Hide resolved
packages/base/src/dialogs/symbology/components/color_ramp/ColorRamp.tsx
Outdated
Show resolved
Hide resolved
packages/base/src/dialogs/symbology/components/color_ramp/ModeSelectRow.tsx
Show resolved
Hide resolved
|
|
||
| useEffect(() => { | ||
| if (selectedRamp === '' && selectedMode === '' && numberOfShades === '') { | ||
| if (selectedRamp === '') { |
There was a problem hiding this comment.
Should we still check if selectedMode and numberofShades are undefined here?
There was a problem hiding this comment.
Sure we can check if selectedMode and numberofShades are undefined here as it is More defensive ensures all three are unset before populating.
packages/base/src/dialogs/symbology/tiff_layer/types/SingleBandPseudoColor.tsx
Outdated
Show resolved
Hide resolved
packages/base/src/dialogs/symbology/vector_layer/types/Graduated.tsx
Outdated
Show resolved
Hide resolved
packages/base/src/dialogs/symbology/vector_layer/types/Graduated.tsx
Outdated
Show resolved
Hide resolved
packages/base/src/dialogs/symbology/components/color_ramp/ColorRamp.tsx
Outdated
Show resolved
Hide resolved
| selectedMode: ClassificationMode | undefined, | ||
| numberOfShades: number | undefined, |
There was a problem hiding this comment.
| selectedMode: ClassificationMode | undefined, | |
| numberOfShades: number | undefined, | |
| selectedMode: ClassificationMode, | |
| numberOfShades: number, |
Let's make these required and check that these are defined before allowing the user to trigger this function to be called (e.g. don't allow the "Classify" button to be clicked unless these are defined). Then we can remove the casts below!
| selectedRamp: string; | ||
| selectedRamp: ColorRampName; |
There was a problem hiding this comment.
What do you think with this change. I think using ColorRampName instead of only type string is a good option.
|
@nakul-py this conflicts with your "default classes" PR that was merged, could you please rebase this and fix conflicts? |
|
Hey! @martinRenou I have resolved conflicts in |
3 participants
Description
Adding typing to useState hooks and Disable the Classify button if user does not provide valid number or negative.
Checklist
Resolves #XXX.Failing lint checks can be resolved with:
pre-commit run --all-filesjlpm run lint📚 Documentation preview: https://jupytergis--937.org.readthedocs.build/en/937/
💡 JupyterLite preview: https://jupytergis--937.org.readthedocs.build/en/937/lite