SpecialAnnouncement to promote teacher applications #19922
Conversation
Erin007
changed the title
| fontSize: 24, | ||
| lineHeight: '26px', | ||
| fontFamily: 'Gotham 3r', | ||
| color: color.charcoal | ||
| }, | ||
| textItem: { | ||
| backgroundColor: color.teal, | ||
| padding: 30, | ||
| paddingLeft: 30, |
There was a problem hiding this comment.
padding adjustments could change depending on copy length
| image: { | ||
| width: 485, | ||
| height: 260 | ||
| }, |
There was a problem hiding this comment.
this seem redundant, but was necessary to constrain the image size on /yourschool because pegasus serves the image differently than dashboard
| return ( | ||
| <div style={styles.fullWidthNonResponsive}> |
There was a problem hiding this comment.
This styling was the only thing really differentiating SpecialAnnouncementActionBlock from TwoColumnActionBlock and we want the special announcement to be responsive so I removed it. Then, I customized SpecialAnnouncementActionBlock in the pattern of the other variations.
|
A note about responsivity: pegasus handles it differently than dashboard and since the page widths are controlled by redux, they don't update on the fly on /yourschool. This means if you view /yourschool special announcement on mobile it looks fine, but if you resize from desktop it doesn't look great. Given we're not expecting that page to be re-sized nor does it get much traffic, I'm considering this non-blocking for now. Seem fair? |
apps/i18n/common/en_us.json
Outdated
| @@ -881,8 +881,8 @@ | |||
| "signinOrAge": "Sign in or provide your age to continue", | |||
| "skipPuzzle": "Skip puzzle", | |||
| "some": "Some", | |||
| "specialAnnouncementHeading": "The Hour of Code is coming", | |||
| "specialAnnouncementDescription": "Try our brand new Minecraft activity with your class for the Hour of Code (Dec 4-10)! This year, we’ve upgraded our servers so your students can play it signed in. They can save their progress and keep their final projects. Sign up your class today!", | |||
| "specialAnnouncementHeading": "Apply for TeacherCon 2018", | |||
There was a problem hiding this comment.
I think we should probably use different key names for different special announcements. One reason is that, in case we start using them for non-en, we don't have old translations showing for new announcements.
There was a problem hiding this comment.
Documenting our offline conversation - we learned that when we change a string it will default back to English until it is translated rather than stay as the translated old string. Also, we don't need to store old announcement text, so we can just change the strings and keep the keeps.
| showInitialTips={showInitialTips} | ||
| userId={userId} | ||
| /> | ||
| <span> |
There was a problem hiding this comment.
Why span? div seems more typical.. I think spans are just for inline elements.
| @@ -81,7 +82,7 @@ class UnconnectedTwoColumnActionBlock extends Component { | |||
| <div style={styles.container}> | |||
| {responsiveSize === 'lg' && | |||
| <div style={{float, width}}> | |||
| <img src={imageUrl}/> | |||
| <img src={imageUrl} style={styles.image}/> | |||
There was a problem hiding this comment.
nit: Do we wrap when there's more than one parameter?
| @@ -140,21 +133,8 @@ export default class TeacherHomepage extends Component { | |||
| <ProtectedStatefulDiv | |||
| ref="termsReminder" | |||
| /> | |||
|
|
|||
There was a problem hiding this comment.
I'm going to do a follow up PR that cleans up our announcement system on this page (takes out DCDO census and hoc_launch conditionals and sets up a clean way to show the small announcement or the big announcement). To keep this change un-blocked I'm just completely removing the small Notification for now.
Erin007
changed the title
| const { ncesSchoolId, censusQuestion, schoolYear } = this.props; | ||
| const { teacherId, teacherName, teacherEmail } = this.props; | ||
| const { canViewAdvancedTools } = this.props; | ||
| const { canViewAdvancedTools, isEnglish, queryStringOpen } = this.props; |
There was a problem hiding this comment.
Out of scope for this change, but just noticed queryStringOpen and can't tell what it does from its name :)
There was a problem hiding this comment.
It's related to managing sections: b1111ed
This PR adds a large Special Announcement banner to /home for teachers, the teacher view of /courses, and /yourschool to promote CSD and CSP professional learning applications. This special announcement is only shown when the user is viewing /courses or /home in English as it is primarily applicable to US teachers.
desktop:
mobile:
Note, this PR also removes the smaller announcement (
Notificationcomponent) from the teacher homepage.