-
Notifications
You must be signed in to change notification settings - Fork 0
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
Modify preview-card styles #88
Changes from 2 commits
5bb4663
ce37ecc
d0756aa
edac142
42d25f7
d268bce
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 |
---|---|---|
@@ -1,11 +1,24 @@ | ||
import { css } from "styled-components"; | ||
|
||
export const exerciseStyles = css` | ||
|
||
&.included-card { | ||
background-color: #daf3f8; | ||
jivey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
.step-card-footer-inner, | ||
.step-card-body, | ||
.step-card-header, | ||
.answer-letter-wrapper::before { | ||
background-color: #daf3f8 !important; | ||
} | ||
} | ||
|
||
&.preview-card { | ||
--spacing: 0.8rem; | ||
|
||
.step-card-header, | ||
.step-card-body { | ||
background-color: #FFFFFF; | ||
padding: var(--spacing); | ||
font-size: 1.6rem; | ||
line-height: 2rem; | ||
|
@@ -44,6 +57,7 @@ export const exerciseStyles = css` | |
.openstax-question { | ||
.openstax-answer { | ||
padding: 0; | ||
border: none; | ||
|
||
.answer-label { | ||
padding-top: var(--spacing); | ||
|
@@ -74,13 +88,31 @@ export const exerciseStyles = css` | |
} | ||
} | ||
|
||
.step-card-footer { | ||
.step-card-footer, .detailed-solution { | ||
display: none; | ||
} | ||
|
||
.question-stem, | ||
.question-feedback-content { | ||
.question-feedback-content, | ||
.question-info, | ||
.exercise-context { | ||
line-height: 2rem; | ||
} | ||
|
||
.question-info { | ||
font-weight: bold; | ||
font-size: 1.6rem; | ||
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. Does this match Figma? I think Figma might be slightly smaller. Also I'm not sure when that other PR will get merged, so maybe we do include the fix you made last week for the broken separator :D 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. You're right, it should be 1.2rem |
||
} | ||
|
||
.question-id { | ||
font-weight: 400; | ||
font-size: 1.6rem; | ||
} | ||
|
||
.question-stem { | ||
color: #424242; | ||
jivey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
font-weight: bold; | ||
font-size: 1.6rem; | ||
} | ||
} | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
import { ExercisePreview } from './ExercisePreview'; | ||
import renderer from 'react-test-renderer'; | ||
import { ExerciseData } from 'src/types'; | ||
|
||
describe('ExercisePreview', () => { | ||
describe('using step data', () => { | ||
|
||
let exercise: ExerciseData; | ||
|
||
beforeEach(() => { | ||
exercise = { | ||
uid: '1@1', | ||
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 think these are indented 4 but we use 2 in this repo (and most of the repos) |
||
uuid: 'e4e27897-4abc-40d3-8565-5def31795edc', | ||
group_uuid: '20e82bf6-232e-40c8-ba68-2d22c6498f69', | ||
number: 1, | ||
version: 1, | ||
published_at: '2022-09-06T20:32:21.981Z', | ||
context: 'Context', | ||
stimulus_html: '<b>Stimulus HTML</b>', | ||
tags: [], | ||
authors: [{ user_id: 1, name: 'OpenStax' }], | ||
copyright_holders: [{ user_id: 1, name: 'OpenStax' }], | ||
derived_from: [], | ||
is_vocab: false, | ||
solutions_are_public: false, | ||
versions: [1], | ||
questions: [{ | ||
id: '1234@5', | ||
collaborator_solutions: [], | ||
formats: ['true-false'], | ||
stimulus_html: '', | ||
stem_html: '', | ||
is_answer_order_important: false, | ||
answers: [{ | ||
id: '1', | ||
correctness: undefined, | ||
content_html: 'True', | ||
}, { | ||
id: '2', | ||
correctness: undefined, | ||
content_html: 'False', | ||
}], | ||
}], | ||
}; | ||
}); | ||
|
||
it.each` | ||
enableOverlay | description | ||
${true} | ${'with overlay'} | ||
${false} | ${'without overlay'} | ||
`('matches snapshot %description', ({ enableOverlay }: { enableOverlay: boolean }) => { | ||
const selectedQuestionsMock: string[] = []; | ||
const setQuestionsmock = jest.fn(); | ||
const tree = renderer.create( | ||
<ExercisePreview | ||
exercise={exercise} | ||
enableOverlay={enableOverlay} | ||
selectedQuestions={selectedQuestionsMock} | ||
setSelectedQuestions={setQuestionsmock} | ||
/> | ||
).toJSON(); | ||
expect(tree).toMatchSnapshot(); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
import React, { useCallback } from "react"; | ||
import { ExerciseData, ExerciseQuestionData, StepBase } from "src/types"; | ||
import { IncludeRemoveQuestion } from "./IncludeRemoveQuestion"; | ||
import { Exercise } from "./Exercise"; | ||
|
||
export interface ExercisePreviewProps { | ||
exercise: ExerciseData; | ||
selectedQuestions: string[]; | ||
setSelectedQuestions?: React.Dispatch<React.SetStateAction<string[]>>; | ||
enableOverlay?: boolean; | ||
} | ||
|
||
/** | ||
* An Exercise version with less interaction with card and grants an Overlay with Include/Remove component | ||
*/ | ||
export const ExercisePreview = ( | ||
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'd prefer this to take |
||
{ | ||
exercise, | ||
selectedQuestions, | ||
setSelectedQuestions, | ||
enableOverlay = false, | ||
}: ExercisePreviewProps) => { | ||
|
||
const exercisePreviewProps = (exercise: ExerciseData) => { | ||
const formatAnswerData = (questions: ExerciseQuestionData[]) => questions.map((q) => ( | ||
{ id: q.id, correct_answer_id: (q.answers.find((a) => a.correctness === '1.0')?.id || '') })); | ||
|
||
const questionStateFields = { | ||
available_points: '1.0', | ||
is_completed: true, | ||
answer_id: '1', | ||
free_response: '', | ||
feedback_html: '', | ||
correct_answer_feedback_html: '', | ||
attempts_remaining: 0, | ||
attempt_number: 1, | ||
incorrectAnswerId: 0 | ||
} | ||
|
||
const questionStates = formatAnswerData(exercise.questions).reduce((acc, answer) => { | ||
const { id, correct_answer_id } = answer; | ||
return { ...acc, [id]: { ...questionStateFields, correct_answer_id } }; | ||
}, {}); | ||
|
||
const step: StepBase = { | ||
id: 1, | ||
uid: exercise.uid, | ||
available_points: '1.0', | ||
}; | ||
|
||
return { | ||
canAnswer: true, | ||
needsSaved: true, | ||
hasMultipleAttempts: false, | ||
onAnswerChange: () => undefined, | ||
onAnswerSave: () => undefined, | ||
onNextStep: () => undefined, | ||
apiIsPending: false, | ||
canUpdateCurrentStep: false, | ||
step: step, | ||
questionNumber: exercise.number as number, | ||
numberOfQuestions: exercise.questions.length, | ||
questionStates: questionStates, | ||
show_all_feedback: false, // Hide all feedback | ||
}; | ||
}; | ||
|
||
const includeQuestionHandler = React.useCallback(() => { | ||
setSelectedQuestions?.(previous => previous.concat(exercise.uid)); | ||
}, [exercise, setSelectedQuestions]); | ||
|
||
const removeQuestionHandler = useCallback(() => { | ||
setSelectedQuestions?.(previous => previous.filter((id) => id !== exercise.uid)); | ||
}, [exercise, setSelectedQuestions]); | ||
|
||
const includeRemoveQuestionComponent = React.useMemo(() => | ||
<IncludeRemoveQuestion | ||
buttonVariant={selectedQuestions.includes(exercise.uid) ? 'remove' : 'include'} | ||
onIncludeHandler={includeQuestionHandler} | ||
onRemoveHandler={removeQuestionHandler} | ||
/> | ||
, [selectedQuestions, exercise, includeQuestionHandler, removeQuestionHandler]); | ||
|
||
return ( | ||
<Exercise | ||
exercise={exercise} | ||
className={selectedQuestions.includes(exercise.uid) ? 'preview-card included-card' : 'preview-card'} | ||
{ | ||
...(enableOverlay | ||
? { | ||
overlayChildren: includeRemoveQuestionComponent, | ||
} | ||
: {}) | ||
} | ||
{...exercisePreviewProps(exercise)} | ||
/> | ||
); | ||
}; |
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.
Does this break any of the other stories? Seems alright, I just can't remember why it was undefined originally
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 marks a question as correct, I can keep it as undefined and create a new variable just for new stories