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
Finalize on TeacherDashboardHeader after A/B test #33830
Conversation
Consolidate TeacherDashboardHeader and TeacherDashboardHeaderWithButtons. Keep only variant that features buttons, and a 'switch section' button to switch sections
<span style={styles.sectionPrompt}> | ||
{i18n.assignedToWithColon()}{' '} | ||
</span> | ||
{this.props.selectedSectionScript.name} |
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.
You could combine these strings and pass the script name as a variable like this:
code-dot-org/apps/i18n/common/en_us.json
Line 95 in 615055f
"assignConfirm": "Are you sure you want to assign \"{assignmentName}\" to \"{sectionName}\"?", |
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.
Will ask you more about this over slack
}} | ||
icon="gear" | ||
size="narrow" | ||
color="gray" |
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 we use the Button component's specified gray just in case we decide to change that default down the road? Like this:
color={Button.ButtonColor.gray} |
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.
I will implement this in a follow up PR! trying to get this merged asap right now, ty for the pointer
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.
Yay! So excited to see us make data-driven decisions that increase usage. A couple of very minor tidying suggestions, but looks great!
} | ||
|
||
getDropdownOptions(optionMetricName) { | ||
let self = this; |
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.
I haven't seen the self = this
pattern in a while.
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.
🎉
Consolidate TeacherDashboardHeader and TeacherDashboardHeaderWithButtons.
Keep only variant that features buttons, and a 'switch section' button to switch sections.
We ran this A/B test for a few weeks. The final results are here. Amanda was able to significantly improve the engagement on 'switching sections', after seeing the result of the first round of A/B tests.
Links
Testing story
Unit tests.
Reviewer Checklist: