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

Remove Watch Video button from mozfest homepage #3900

Closed
gideonthomas opened this issue Nov 14, 2019 · 8 comments
Closed

Remove Watch Video button from mozfest homepage #3900

gideonthomas opened this issue Nov 14, 2019 · 8 comments
Milestone

Comments

@gideonthomas
Copy link
Contributor

Remove the button at the top of the site. Bonus: make it toggleable via the CMS so that we can easily add it back in later.

@gideonthomas gideonthomas added this to the Nov 25 milestone Nov 14, 2019
@ragnarokatz
Copy link
Contributor

@gideonthomas Hello, I would like to work on this issue please.

@ragnarokatz
Copy link
Contributor

ragnarokatz commented Nov 14, 2019

I'm not sure if this is what you mean by making it toggleable via CMS:

in template file partials/homepage_banner.html:

      {% if page.banner_video_url %}
      <a class="btn btn-primary" href="{{ page.banner_video_url }}" target="_blank" id="mozfest-home-watch-video-button">Watch Video</a>
      {% endif %}

there's a toggle available here, that if page.banner_video_url is passed in, then the watch video button will show up.

It is being passed in to the template in file mozfest/models.py:

    banner_video_url = models.URLField(
        max_length=2048,
        blank=True,
        help_text='The video to play when users click "watch video"'
    )

...

    content_panels = parent_panels[:n] + [
        FieldPanel('cta_button_label'),
        FieldPanel('cta_button_destination'),
        FieldPanel('banner_heading'),
        FieldPanel('banner_guide_text'),
        FieldPanel('banner_video_url'),
    ] + parent_panels[n:] + [
        FieldPanel('prefooter_text'),
    ]

Should I just remove the FieldPanel('banner_video_url') here?

@gideonthomas
Copy link
Contributor Author

@Pomax are you ok with just commenting out that button in the template and adding a comment saying that we will probably add it back next year?

@ragnarokatz
Copy link
Contributor

ragnarokatz commented Nov 16, 2019

while we're waiting on a response, should I just create a PR with what you have recommended? as in "just commenting out that button in the template and adding a comment saying that we will probably add it back next year" @gideonthomas

@gideonthomas
Copy link
Contributor Author

yes please! Thanks @ragnarokatz

@ragnarokatz
Copy link
Contributor

@gideonthomas has been created and ready for review.

@Pomax
Copy link
Contributor

Pomax commented Nov 19, 2019

@ragnarokatz's observation that we have a URL field that we can treat as toggle looks sound to me, I don't see a reason not to just immediately go for "if url is set, show, otherwise, don't".

@ragnarokatz
Copy link
Contributor

@gideonthomas Since #3910 has been closed, I believe this ticket can be resolved as well.

@alanmoo alanmoo closed this as completed Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants