Skip to content
This repository has been archived by the owner on Sep 21, 2023. It is now read-only.

feature(KickerSection): Introduce Kicker Section #831

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

misund
Copy link
Contributor

@misund misund commented Dec 5, 2018

Work in progress

This PR contains several features, and should probably be rebased onto master instead of squashed.

Please tick a box

  • feature (A new feature)
  • fix (A bug fix)
  • refactor (A code change that neither fixes a bug nor adds a feature)
  • docs (Documentation only changes)
  • performance (A code change that improves performance)
  • style (Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc))
  • build (Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm))
  • ci (Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs))
  • test (Adding missing tests or correcting existing tests)

If you're adding a feature, is it documented in a storybook story?

  • Yes
  • No
  • Not adding a new feature

Are the components you're working on reusable between brands?

  • Yes (Using variables that are defined in the default theme)
  • No (I hard coded some values)
  • Not working on any components

If you're creating markup, did you add proper semantics?

(Did you do a CR and see that there is something that we should check for each PR, that are not on the list, please update this document)

@misund misund changed the title eature(KickerSection): Introduce Kicker Section feature(KickerSection): Introduce Kicker Section Dec 5, 2018
@misund misund force-pushed the feature/kicker-section branch from 69ddc32 to d415950 Compare December 5, 2018 16:55
@misund misund force-pushed the feature/kicker-section branch from 531db7c to b6cf719 Compare December 5, 2018 18:54
alt={title}
ratio={ratio}
offset={offset}
src={placeholderUrl ? placeholderUrl : image}
preventBlur={preventBlur}
fadeIn={fadeIn}
>
{displayPlayIcon && <PlayIcon />}
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to have a color prop for icon

Copy link
Contributor Author

@misund misund Dec 6, 2018

Choose a reason for hiding this comment

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

PlayIcon will pass props along to SvgIcon


import { OverlayingPlugLabel } from '../atoms/OverlayingPlugLabel';

const Footer = styled.footer`
Copy link
Contributor

Choose a reason for hiding this comment

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

footer? its for seo or smth?

Copy link
Contributor Author

@misund misund Dec 6, 2018

Choose a reason for hiding this comment

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

I needed an enclosing element to avoid individual labels taking part in the PlugLink's column-direction flex – if they did, they would fill the entire width of the plug. The PlugLink's column-direction flex is there to give us the the order attribute. When adding the extra element anyways, we might as well try to add some semantics.

The footer element represents a footer for its nearest ancestor sectioning content or sectioning root element.

☝️ HTML spec. It's nearest ancestor sectioning content would be our <article> element.

A footer typically contains information about its section such as who wrote it, links to related documents, copyright data, and the like.

Basically, a <footer> is a good place to put metadata that does not go in a <header>.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants