Skip to content
This repository has been archived by the owner on Nov 10, 2021. It is now read-only.

[RHDX-221] Add webinar countdown component #3496

Merged

Conversation

staceymosier
Copy link
Contributor

@staceymosier staceymosier commented Feb 6, 2020

JIRA Issue Link

RHDX-221

Verification Process

View sample webinar page here: https://developer-preview-3496.ext.us-west.dc.preprod.paas.redhat.com/node/214685/

Scenario 1
Create a new page with a Call to Action assembly, or edit a Call to Action assembly.
Provide a value for CTA Timer and CTA Timer Label for a few minutes in the future.
Use the Left visual styles so that content flushes left. Use the Dark visual style and provide a dark background image.
Verify the Countdown is accurate for your timezone.
At 0, it should be hidden and the CTA button should appear.

Scenario 2
Verify the Call to Action assemblies without Countdown values still work as expected.

Scenario 3
Edit the CTA from Scenario 1 and remove the Left visual style.
Check how it renders as centered. Check how it renders a Light visual style and light background image.

Note: This assembly is using pf-l-grid 8 columns to keep the text from overflowing the lightbulb.
https://developer-preview-3496.ext.us-west.dc.preprod.paas.redhat.com/node/214685/

@staceymosier staceymosier self-assigned this Feb 6, 2020
@staceymosier staceymosier changed the title [RHDX-221] Add webinar countdown component [Do Not Merge] [RHDX-221] Add webinar countdown component Feb 7, 2020
@staceymosier
Copy link
Contributor Author

rebuild this please

@staceymosier staceymosier changed the title [Do Not Merge] [RHDX-221] Add webinar countdown component [RHDX-221] Add webinar countdown component Feb 10, 2020
Copy link

@gdoyle1 gdoyle1 left a comment

Choose a reason for hiding this comment

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

@staceymosier this is looking good! there's a few things I found:

  1. Scenario 1 -- looks great
  2. Scenario 2 -- looks and works as expected
  3. Scenario 3 -- when the "left" visual style is removed from the assembly, the content is then centered. we should have the countdown centered as well (the CTA in that centered styled assembly is also centered currently)
  4. I created a scenario of using it in a rich text assembly (you'll see it in the 4th assembly i added to your test page - also in the screenshot below). I copied the code you added for the countdown and put it into a grid. It looks like the countdown component was added but the timer doesn't work. If there's a possibility that we can be able to copy and paste that component into a rich text assembly, that's what I would ask for in addition to what you have already done.

Scenarios 1-3:
Screen Shot 2020-02-10 at 11 30 56 AM

Rich text assembly:
Screen Shot 2020-02-10 at 11 34 38 AM

@jordanpagewhite
Copy link
Contributor

@gdoyle1 and @staceymosier Re: #4 in Gina's comment above,

I think that we should explore what use cases exist that could require this countdown component to be placed in a Rich Text assembly. I assume that these scenarios are motivated by layout possibilities? Are there additional concerns? We should definitely discuss these. I would like to identify and fully define the problem before we discuss potential solutions.

@jordanpagewhite
Copy link
Contributor

Also, I reviewed the current state of the code and PR environment, and the changes look good to me, but I will wait to approve this until we have resolved Gina's comment 4 above.

@staceymosier
Copy link
Contributor Author

@gdoyle1 I pushed up a change to fix the center display on the default assembly (i.e. without the left visual style). I also checked it on mobile; the max width it will grow to be is 400px.

I did not add the countdown to the rich text assembly. The call to action assembly has the 'call to action' link field that can be targeted specifically to hide/show. The editor has the option of putting link/button markup in the body of the cta field, however it will appear before the countdown.

When the PR rebuilds, I'll put the sample content back and request another review.

@gdoyle1
Copy link

gdoyle1 commented Feb 10, 2020

@gdoyle1 and @staceymosier Re: #4 in Gina's comment above,

I think that we should explore what use cases exist that could require this countdown component to be placed in a Rich Text assembly. I assume that these scenarios are motivated by layout possibilities? Are there additional concerns? We should definitely discuss these. I would like to identify and fully define the problem before we discuss potential solutions.

@jordanpagewhite @staceymosier I think SJ and I originally thought we'd want to place it into a rich text assembly since that provides the most flexibility as a content creator BUT thinking about the use cases where the countdown component will be used, it will just be for these webinar pages. With that said, everything that was recreated in the sample page using the Call to action assembly was exactly how it is in Drupal. So, after I make sure the center visual style works, I think we can merge this from a UX standpoint!

@sjcox-rh if you could verify that that is correct too that'd be great ^

@sjcox-rh
Copy link

@gdoyle1 you are correct. We originally requested that we have this feature in all 3 rich text components BUT Gina made a good point that we could still create the same experience with the call to action assembly so I'm okay with merging it as it stands.

If we get to a point in the feature where we would need this component in more assemblies, then we can just revisit this.

@staceymosier I did have a tiny question. Does this ticket as address the behavior what happens when the countdown ends?

Will it turn into the CTA?

Copy link
Contributor

@jordanpagewhite jordanpagewhite left a comment

Choose a reason for hiding this comment

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

I reviewed this PR this morning but forgot to actually add my review. These changes look good to me. I changed my user's timezone to UTC, as stated in the PR summary, to get the timer working as intended.

Copy link

@sjcox-rh sjcox-rh left a comment

Choose a reason for hiding this comment

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

Reviewed and everything seemed to be working as intended!

@staceymosier staceymosier merged commit c8678f4 into redhat-developer:master Feb 28, 2020
@staceymosier staceymosier deleted the RHDX-221--webinar-countdown branch March 4, 2020 22:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants