-
Notifications
You must be signed in to change notification settings - Fork 167
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
Grading Overview: TS/Tremor -> AG/Blueprint Migration #2893
Changes from 17 commits
97a1ae4
567a471
0c5df89
5a5eaab
852da3c
c4f0ddf
34f8d9a
b22cb6f
30014c8
63932c6
5a4eb0d
de27b6d
48684a5
1e7eafc
f7021b0
e54fe6e
6ae6df5
c16e3ae
d1d4a09
2db1749
83f00cf
f1f8652
2ed0eec
2d5cae3
886c976
d901660
768cb8a
20fe50a
ca6ac7e
b73044b
651e457
2dc8125
480bbfc
2aa51e5
9dad80b
f608c87
e849778
fef4031
3c4c939
d57f0d8
4b76614
3c5edf1
fbba60a
b03cfe9
9b2348e
505de5f
d161760
a4ff982
db2961c
e93a8d5
04cc13e
8b02ad1
86a431b
c4134c0
f7ea428
7449edf
5428c57
ceebfb9
916680c
8a1cad8
847a6ab
1f4108f
d2756bc
71eb6d6
a14c365
70fbc0f
3d1ec43
0d91ae8
f79be42
da2e269
fa7a309
14b8360
e8e68c5
72f301f
e08d9b5
086460d
5f88cd2
a858c18
f02d792
f6664ac
c9bf19c
6d21dca
e48bb8f
d2db6d2
057dd5a
74aa4e7
9544a0c
21fb5f6
286a204
de628df
23d330f
24fffed
5c95916
8960ade
874ae71
1b32f44
3abcdad
248c03f
fedce2a
9bc2656
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 |
---|---|---|
@@ -0,0 +1,56 @@ | ||
declare const twJustifyContentValues: readonly ["justify-start", "justify-end", "justify-center", "justify-between", "justify-around", "justify-evenly"]; | ||
declare type JustifyContent = typeof twJustifyContentValues[number]; | ||
declare const twAlignItemsValues: readonly ["items-start", "items-end", "items-center", "items-baseline", "items-stretch"]; | ||
declare type AlignItems = typeof twAlignItemsValues[number]; | ||
declare const twFlexDirectionValues: readonly ["row", "column"]; | ||
declare type FlexDirection = typeof twFlexDirectionValues[number]; | ||
|
||
type GradingFlexProps = { | ||
justifyContent?: JustifyContent; | ||
alignItems?: AlignItems; | ||
flexDirection?: FlexDirection; | ||
children?: React.ReactNode; | ||
style?: React.CSSProperties; | ||
className?: string; | ||
} & React.RefAttributes<HTMLDivElement>; | ||
|
||
const GradingFlex: React.FC<GradingFlexProps> = ({ justifyContent, alignItems, flexDirection, children, style, className, }: GradingFlexProps) => { | ||
const defaultStyle: React.CSSProperties = { | ||
display: "flex", | ||
justifyContent: | ||
(justifyContent === "justify-start" | ||
? "flex-start" | ||
: justifyContent === "justify-end" | ||
? "flex-end" | ||
: justifyContent === "justify-center" | ||
? "center" | ||
: justifyContent === "justify-between" | ||
? "space-between" | ||
: justifyContent === "justify-around" | ||
? "space-around" | ||
: justifyContent === "justify-evenly" | ||
? "space-evenly" | ||
: "" | ||
), | ||
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. Seems like we are mapping Tailwind values to actual styles – what's the need for this? Can we just map input props directly? 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 was just being lazy and trying to avoid touching existing props but I can go through all of it again when I work on it to remove it. 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 has been fixed in the latest commit, it can be resolved once you are ok with it. |
||
alignItems: | ||
(alignItems === "items-start" | ||
? "start" | ||
: alignItems === "items-end" | ||
? "end" | ||
: alignItems === "items-center" | ||
? "center" | ||
: alignItems === "items-baseline" | ||
? "baseline" | ||
: "" | ||
), | ||
flexDirection: flexDirection, | ||
}; | ||
|
||
return ( | ||
<div className={className} style={{...defaultStyle, ...style}}> | ||
{children} | ||
</div> | ||
); | ||
} | ||
|
||
export default GradingFlex; | ||
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. Please add an empty line at the end of this file. 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 has been fixed in the latest commit, it can be resolved once you are ok with it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import { Text } from "@blueprintjs/core"; | ||
|
||
type GradingTextProps = { | ||
children?: React.ReactNode; | ||
style?: React.CSSProperties; | ||
secondaryText?: boolean; | ||
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. Nit: booleans should be named |
||
className?: string; | ||
} & React.RefAttributes<HTMLDivElement>; | ||
|
||
const GradingText: React.FC<GradingTextProps> = ({ children, style, secondaryText, className = "", }: GradingTextProps) => { | ||
const defaultStyle: React.CSSProperties = { | ||
width: "max-content", | ||
margin: "auto 0" | ||
}; | ||
|
||
return ( | ||
<Text | ||
className={"bp5-ui-text " + className + (secondaryText ? " bp5-text-muted" : "")} | ||
style={{...defaultStyle, ...style}} | ||
> | ||
{children} | ||
</Text> | ||
); | ||
} | ||
|
||
export default GradingText; | ||
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. Ditto on newline at EOF 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 has been fixed in the latest commit, it can be resolved once you are ok with it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -457,7 +457,25 @@ function* BackendSaga(): SagaIterator { | |
return; | ||
} | ||
|
||
const { filterToGroup, gradedFilter, pageParams, filterParams } = action.payload; | ||
const { filterToGroup, gradedFilter, pageParams, filterParams, allColsSortStates } = | ||
action.payload; | ||
|
||
const sortedBy = { | ||
sortBy: allColsSortStates.sortBy, | ||
sortDirection: '' | ||
}; | ||
|
||
for (const key in allColsSortStates.currentState) { | ||
if (allColsSortStates.sortBy === key) { | ||
if (allColsSortStates.currentState[key] !== 'sort') { | ||
sortedBy.sortDirection = allColsSortStates.currentState[key]; | ||
} else { | ||
sortedBy.sortBy = ''; | ||
sortedBy.sortDirection = ''; | ||
} | ||
break; | ||
} | ||
} | ||
|
||
const gradingOverviews: GradingOverviews | null = yield call( | ||
getGradingOverviews, | ||
|
@@ -466,6 +484,7 @@ function* BackendSaga(): SagaIterator { | |
gradedFilter, | ||
pageParams, | ||
filterParams | ||
// sortedBy | ||
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. Is this going to be used/removed? 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. It will be used for backend column sorting onced implemented, commented for now in case it causes errors. 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 has been uncommented in the latest commit and the various issues that comes with it has been fixed. |
||
); | ||
if (gradingOverviews) { | ||
yield put(actions.updateGradingOverviews(gradingOverviews)); | ||
|
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.
Can I check what is this used for?
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.
think I copied it blindly to avoid errors, will look through as well with the other next comment when I get around to looking at it.
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 has been fixed in the latest commit, it can be resolved once you are ok with it.