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

EM-1241: Why CB Solutions Section (ESB) #127

Merged
merged 10 commits into from
Aug 29, 2017

Conversation

mrfantasticwonders
Copy link
Contributor

@mrfantasticwonders mrfantasticwonders commented Aug 25, 2017

Purpose

As a potential buyer or high-level decision maker, I want to visit Hiring Dot and know what CareerBuilder can offer me as a vendor/partner.

This PR adds in the card-group component mixin for the card-group app. It also adds in a "gating" sheet to limit styles for some browsers.

JIRA
https://cb-content-enablement.atlassian.net/browse/EM-1241

Changes

  • Improvements and fixes

    • This introduces the card_group component and gating utility mixins
  • Changes to developer setup/environment

    • N/A
  • Architectural changes

    • Adds card_group.scss component mixin
    • Adds gating.scss utility mixin
  • Migrations or Steps to Take on Production

    • N/A
  • Library changes

    • N/A
  • Side effects

    • N/A

Screenshots
screen shot 2017-08-23 at 3 37 41 pm

Feature Server
http://web.employer-2.development.c66.me/company/why-careerbuilder
http://dev.admin.cbcortex.com/legacy#/webpages/24

How to Verify These Changes

  • Specific pages to visit

    • See the QA links above
  • Steps to take

    • Go the Employer link above first and make sure the cards resemble and act like those found on https://hiring.careerbuilder.com/recruiting-solutions/small-business-subscription-plans-and-pricing#small-biz__add-ons (the Add Ons section).

      • Make sure the hover animation is still present
      • Resize to mobile, the cards should stack similarly to the HiringDot version.
      • The logos should change color on hover
    • Click on a card, it should flip over. Check styling found in the Invision links in Jira.

    • Click outside of the card, it should reset.

    • Click on a card, then click on another card - the first card you clicked on should reset.

    • Go to Cortex using the link above to edit this page. You should immediately notice that both the front and back sides of each cards is displayed. If you click on the cards (outside of IPE), animations should not trigger.

    • Try editing the headers, body text, both of the buttons, and the list items. The web page in Cortex should update with both sides of the cards visible. Check the actual page, your changes should be seen, but with the flipping functionality enabled.

  • Responsive considerations

    • Be sure to check the cards on your phone / tablet. It should only take one click to flip the cards over. Additionally, there is no hover state present for the iPad and iPhone - this is because of the double tap issue with both those phones.

Relevant PRs/Dependencies

Additional Information
Card height - One of you will probably add in a bunch of text that will cause content in the card to overflow. You'll notice right away that the height of the cards is fixed. This might warrant another story to make the height dynamic. The reason we need a fixed height is because the flipping functionality uses absolutely positioned elements that causes the cards to collapse. Talked with @arelia about this, and for now, this will be the solution.

We might also need to consult @justin3thompson to figure out how many items he intends to show on the back of the cards.

@@ -0,0 +1,25 @@
@mixin card-group-gate {
Copy link
Contributor

Choose a reason for hiding this comment

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

since data-platform is specific to Employer (and I think it should stay specific to Employer, it's not necessarily something to include in style base and encourage others to use for consistency, etc) these selectors should stay in Employer

also, these selectors are specific to cards and small-biz, so they should go in those style sheets

or if you think this is a must-have for making cards work, put the styles there. but when you're adding things to style base think "does a third party have to have this so that their site is consistent with employer?"

Copy link
Contributor Author

@mrfantasticwonders mrfantasticwonders Aug 28, 2017

Choose a reason for hiding this comment

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

So this mixin was split, with _gating.scss being removed. The styles specific to card-group were kept in _card_group.scss since they're very generic and yes, if a 3rd party wants to include the logo w/ hover, it would need to be disabled for the iPhone and iPad due to the doubletap bug.

The other part dealing with the small-biz content (actually it was changed to why-cb) was moved back to employer for the reasons you stated.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 and sorry, this was a bit blunt, I was in a rush to submit this this morning

.card-group__card {
@include card--tile($include_strip: false);
// Restore the original box-shadow since it is lost when adding layers. This is not the same as $base-box-shadow-hover
box-shadow: -1px 2px 1px 0px rgba(0, 0, 0, 0.25);
Copy link
Contributor

Choose a reason for hiding this comment

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

there should be a base-box-shadow variable or something like that, in sizing i think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to make this into a variable actually. The reason for this is that due to layering of these cards, base-box-shadow and base-box-shadow-hover are not the same with these cards compared to our normal cards. Using base-box-shadow with these cards actually produces a shadow that really cannot be seen at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@mrfantasticwonders
Copy link
Contributor Author

mrfantasticwonders commented Aug 28, 2017

@arelia I'll keep this PR open, but I'm moving the Jira into Done for Justin to review (since you approved the Employer side of this PR and due to you being at the conference).

cc @toastercup

@mrfantasticwonders mrfantasticwonders merged commit faacbdd into master Aug 29, 2017
@mrfantasticwonders mrfantasticwonders deleted the topic/EM-1241-WCB-Solutions-Section branch August 29, 2017 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants