Added the fts star to the scoreboard #140
Conversation
Mond1c
changed the title
| width: 33px; | ||
| height: 33px; |
There was a problem hiding this comment.
Do I need to make this value constant? (maybe STAR_SIZE in config.js?)
There was a problem hiding this comment.
Yes, please extract to config.js
| }; | ||
|
|
||
| const AttemptsOrScoreLabelWrapper = styled.div` | ||
| position: relative; |
There was a problem hiding this comment.
Why is there a rouge position: relative?
There was a problem hiding this comment.
Doesn't it work without the position: relative?
There was a problem hiding this comment.
If I remove it, the star will above the text
There was a problem hiding this comment.
Change the order of the elements in the html tree
There was a problem hiding this comment.
I just changed position: relative to position: absolute in the AttemptsOrScoreLabelWrapper div and removed position: absolute from the StarIcon div.
| {problemResult.type === "ICPC" && <ICPCTaskResultLabel2 problemResult={problemResult} {...props}/>} | ||
| {problemResult.type === "IOI" && <IOITaskResultLabel2 problemResult={problemResult} minScore={minScore} maxScore={maxScore} {...props}/>} | ||
| {problemResult.type === "ICPC" && <ICPCTaskResultLabel2 problemColor={problemColor} problemResult={problemResult} {...props}/>} | ||
| {problemResult.type === "IOI" && <IOITaskResultLabel2 problemColor={problemColor} problemResult={problemResult} minScore={minScore} maxScore={maxScore} {...props}/>} |
There was a problem hiding this comment.
No reason to extract problemColor or problemResult or minScore or maxScore from the {...props}.
Just leave {...props}
There was a problem hiding this comment.
I can't extract problemResult because:
problemResult.type === "ICPC"There was a problem hiding this comment.
minScore, maxScore, problemColor are now in props
| </TaskResultLabelWrapper2> | ||
| </>; | ||
| }; | ||
|
|
||
| const IOITaskResultLabel2 = ({ problemResult: r, minScore, maxScore, ...props }) => { | ||
| const IOITaskResultLabel2 = ({ problemColor, problemResult: r, minScore, maxScore, ...props }) => { |
There was a problem hiding this comment.
Is it really necessary to rename problemResult to r?
There was a problem hiding this comment.
I didn't do it. That was before I started pr.
There was a problem hiding this comment.
Then I guess we will refactor it some time in the future.
| @@ -119,7 +119,7 @@ export const ScoreboardRow = ({ teamId, hideTasks, optimismLevel }) => { | |||
| formatPenalty(scoreboardData?.penalty) | |||
| } />} | |||
| {!hideTasks && scoreboardData?.problemResults.map((result, i) => | |||
| <ScoreboardTaskResultLabel problemResult={result} key={i} | |||
| <ScoreboardTaskResultLabel problemResult={result} key={i} problemColor={contestData.problems[i].color} | |||
There was a problem hiding this comment.
Maybe we should create a contest-aware component that will look up the colors from the store, but all we had to do is pass it the problem number?
Something like
export const ProblemLabel = ({problemIndex, className}) => {
const problemData = useSelector((state) => state ...... );
return <TaskResultLabel problemColor={problemData.color} ..... >
}There was a problem hiding this comment.
But we are already getting minScore, maxScore, etc. here. Do we want to create a new component for this?
There was a problem hiding this comment.
Yes. Here we are also getting a lot of other useless data. We should be more specific to make things more pure
| @@ -119,7 +119,7 @@ export const ScoreboardRow = ({ teamId, hideTasks, optimismLevel }) => { | |||
| formatPenalty(scoreboardData?.penalty) | |||
| } />} | |||
| {!hideTasks && scoreboardData?.problemResults.map((result, i) => | |||
| <ScoreboardTaskResultLabel problemResult={result} key={i} problemColor={contestData.problems[i].color} | |||
| <ScoreboardTaskResultLabel problemResult={result} key={i} problemColor={contestData?.problems[i]?.color} | |||
There was a problem hiding this comment.
This is the thing I don't like. Passing too many props from the state in the context that is not suitable. I could understand if this was a specialized component that is extracting data from the store and displaying the data, but when it's used here - I'm not sure how I feel about that.
No description provided.