Skip to content
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

New Participate Page #1748

Merged
merged 27 commits into from Aug 21, 2018
Merged

New Participate Page #1748

merged 27 commits into from Aug 21, 2018

Conversation

gvn
Copy link
Contributor

@gvn gvn commented Aug 16, 2018

Related issue: #1717

@cadecairos cadecairos temporarily deployed to foundation-mofostaging-pr-1748 August 16, 2018 22:58 Inactive
@gvn gvn temporarily deployed to foundation-mofostaging-pr-1748 August 16, 2018 22:59 Inactive
@gvn gvn temporarily deployed to foundation-mofostaging-pr-1748 August 16, 2018 23:01 Inactive
@gvn gvn temporarily deployed to foundation-mofostaging-pr-1748 August 16, 2018 23:27 Inactive
@gvn
Copy link
Contributor Author

gvn commented Aug 17, 2018

@gvn gvn requested a review from kristinashu August 17, 2018 16:40
@kristinashu
Copy link

kristinashu commented Aug 17, 2018

Page is looking good! There are no major issues but here are the P1s:

Share CTA

  • Add FB and Twitter icon to “Spread the word” CTA (this copy will probably updated later)
    Copy: The internet needs your help. From signing a petition, contributing to a project, to attending events, you can make the Internet a better place http://foundation.mozilla.org/participate/

Time commitment tags

  • on Firefox, font in time commitment should be Nunito sans (not serif font in screen cap)

image

Top hero banner (see comp or Abstract)

  • dividing line at bottom of hero banner should be Light Gray ($light-gray) #cccccc instead of the #979797 that is there

image

- [ ] increase space below the yellow button and above the dividing line from mb-3 to mb-4 (nvm)

  • increase margin bottom of diving line from mb-3 to mb-4

Medium width view (see comp or Abstract)

  • Make text go across more of the screen for both the hero text and the intro text below
  • Make hero image full bleed

image

P2's

I will open a separate ticket for the P2's on Monday. For a sneak peak, see this doc.

cc @beccaklam

@gvn gvn temporarily deployed to foundation-mofostaging-pr-1748 August 20, 2018 18:50 Inactive
@gvn gvn temporarily deployed to foundation-mofostaging-pr-1748 August 20, 2018 18:59 Inactive
@gvn
Copy link
Contributor Author

gvn commented Aug 20, 2018

@kristinashu P1s are done and staged. If they look good please mark your review approved and I'll assign a code reviewer.

Let's move P2s to a new ticket. Thanks!

@kristinashu
Copy link

Cool, thanks @gvn! For the share messaging, can I update the pre-populated text somewhere in the CMS?

@kristinashu
Copy link

@gvn if the share messaging isn't editable in the CMS, then can you please use the copy above for all the platforms?

I've opened a new ticket for P2's #1749 and will delete the list above to avoid any confusion.

@gvn
Copy link
Contributor Author

gvn commented Aug 20, 2018

All of the copy is editable in the CMS. Feel free to change it on staging if you'd like!

@kristinashu
Copy link

Whaaaat!? Awesome! Will approve design review now.

Copy link

@kristinashu kristinashu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design is good! Ready for code review.

@gvn gvn requested a review from mmmavis August 20, 2018 20:42
@gvn gvn changed the title New Participate Page (WIP!) New Participate Page Aug 21, 2018
Copy link
Collaborator

@mmmavis mmmavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments and questions.

@@ -494,11 +494,170 @@ class InitiativesPage(PrimaryPage):
]


# TODO: Remove this model after ParticipatePage2 is in use
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will eventually! Not yet though.

ordering = ['sort_order'] # not automatically inherited!

def __str__(self):
return self.page.title + '->' + self.highlight.title
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this appear in UI? I can't seem to find any text on site / admin dashboard that has ->.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH this is copied from elsewhere and seems to be needed, but I'm not sure why. 🤷‍♀️

@@ -623,6 +886,7 @@ class Homepage(MetadataPageMixin, Page):
'Styleguide',
'NewsPage',
'ParticipatePage',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to remove ParticipatePage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will eventually. Right now there will be two ParticipatePage templates so that we don't have to take down the current /participate page while we build the new one on prod.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kk makes sense

<a class="social-link social-link-twitter" href="https://twitter.com/home?status={{ twitter }}"></a>
{% endif %}

{% if email_body %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to check email_subject here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I guess that's a good idea.

@gvn gvn temporarily deployed to foundation-mofostaging-pr-1748 August 21, 2018 20:52 Inactive
@gvn gvn temporarily deployed to foundation-mofostaging-pr-1748 August 21, 2018 21:02 Inactive
@cadecairos cadecairos temporarily deployed to foundation-mofostaging-pr-1748 August 21, 2018 21:14 Inactive
@gvn gvn merged commit 8b108cc into master Aug 21, 2018
gvn added a commit that referenced this pull request Aug 21, 2018
gvn added a commit that referenced this pull request Aug 21, 2018
gvn added a commit that referenced this pull request Aug 21, 2018
@Pomax Pomax deleted the gh-1717 branch October 30, 2018 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants