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

Add GEL Group A breakpoint #349

Merged
merged 4 commits into from
Mar 13, 2019
Merged

Add GEL Group A breakpoint #349

merged 4 commits into from
Mar 13, 2019

Conversation

DenisHdz
Copy link
Contributor

Resolves #348

Overall change: Added breakpoint for missing GEL Group A

Code changes:

  • Added new GEL_GROUP_A_MAX_WIDTH constant
  • Added new FEATURE_PHONE_ONLY breakpoint for 0 - 319 pixels
  • I have assigned myself to this PR and the corresponding issues
  • Tests added for new features
  • Test engineer approval

@DenisHdz DenisHdz requested a review from a team as a code owner March 12, 2019 10:16
@dr3 dr3 self-requested a review March 12, 2019 11:22
dr3
dr3 suggested changes Mar 12, 2019
View reviewed changes
Copy link
Contributor

@dr3 dr3 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey, Apologies, accidentally approved before, this needs change log, package bump etc :)

sareh
sareh suggested changes Mar 12, 2019
View reviewed changes
Copy link
Contributor

@sareh sareh 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! Just some package admin steps needed.

packages/utilities/gel-foundations/src/breakpoints.js Outdated Show resolved Hide resolved
@DenisHdz
Copy link
Contributor Author

Thank you for reminding about the versions. I have to get used to it :)

dr3
dr3 reviewed Mar 12, 2019
View reviewed changes
packages/utilities/gel-foundations/CHANGELOG.md Outdated
| 0.1.4 | [PR#224](https://github.com/BBC-News/psammead/pull/224) Add tests for the exported values, coverage 100% :tada: |
| 0.1.1 | [PR#212](https://github.com/BBC-News/psammead/pull/212) Update package description and README. |
| Version | Description |
| ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ |
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this file got caught by an auto-linter, can you reset the formatting please? 🙈

@DenisHdz DenisHdz added the ws-home Tasks for the WS Home Team label Mar 12, 2019
sareh
sareh approved these changes Mar 12, 2019
View reviewed changes
Copy link
Contributor

@sareh sareh left a comment

Choose a reason for hiding this comment

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

👍 Looks great!

@DenisHdz DenisHdz requested a review from dr3 March 13, 2019 10:14
dr3
dr3 approved these changes Mar 13, 2019
View reviewed changes
Copy link
Contributor

@dr3 dr3 left a comment

Choose a reason for hiding this comment

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

@dr3
Copy link
Contributor

dr3 commented Mar 13, 2019 •
edited
Loading

@DenisBBC Is this PR not in a project? I cant see one attached

Also I have assigned myself to this PR and the corresponding issues has been ticked but you havent 🙈

@DenisHdz
Copy link
Contributor Author

DenisHdz commented Mar 13, 2019 •
edited
Loading

@DenisBBC Is this PR not in a project? I cant see one attached

Also I have assigned myself to this PR and the corresponding issues has been ticked but you havent 🙈

Yes it is, for some reason it wasn't assigned to the WS Off-Forge project.

@DenisHdz DenisHdz self-assigned this Mar 13, 2019
@DenisHdz DenisHdz merged commit 4800a4a into latest Mar 13, 2019
@sareh sareh deleted the gel-group-a-breakpoint branch March 13, 2019 11:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sareh sareh sareh approved these changes

@dr3 dr3 dr3 approved these changes

Assignees

@DenisHdz DenisHdz

Labels
ws-home Tasks for the WS Home Team
Projects
None yet
Milestone
WS - Week 1, Investigate and implemen...
Development

Successfully merging this pull request may close these issues.
4 participants
@DenisHdz @dr3 @sareh @DenisHernandez