Skip to content

Conversation

@tvooo
Copy link

@tvooo tvooo commented Jul 28, 2017

  • Removed BpkParagraph and BpkHeading
  • Added BpkText
  • Updated other Backpack dependencies

@tvooo tvooo requested a review from matthewdavidson July 28, 2017 12:39
Copy link

@matthewdavidson matthewdavidson left a comment

Choose a reason for hiding this comment

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

Looks good to me otherwise.

padding: $bpk-spacing-base 0;
}

&__text {

Choose a reason for hiding this comment

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

. App__text is declared once but used twice (in the react template). Ideally there should be a one to one mapping of a class to a react element. E.g. imagine we wanted to make the heading underlined:

  &__text {
    margin-bottom: $bpk-spacing-sm;
    text-decoration: underline;
  }

We've just inadvertently added underline to the paragraph. I reckon we should be setting the right example in the template.

@tvooo
Copy link
Author

tvooo commented Jul 28, 2017

Is there a way to request a reviewer to update their review? Except maybe: @matthewdavidson do you want to update your review? :D

@tvooo tvooo merged this pull request into master Jul 28, 2017
@matthewdavidson matthewdavidson deleted the bpk-817-deprecate-heading-paragraph branch July 28, 2017 14:49
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.

3 participants