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

PLAT-360_theme_look_feel added new colours, some need updating, updat… #406

Merged
merged 22 commits into from
Aug 10, 2016

Conversation

wagg-matt
Copy link
Contributor

Fixes https://jira.comicrelief.com/browse/

Changes proposed in this pull request

  • will need to update all link classes that have link--inline-[colour] to inline
  • change...

…ed links and removed inline links mixin in favour of a single class, added new buttons to be reviewed by Leigh Hall

…ed links and removed inline links mixin in favour of a single class, added new buttons to be reviewed by Leigh Hall
@wagg-matt
Copy link
Contributor Author

@gusliedke @AndyEPhipps not sure if you guys fancy a look at this on the way through and if there's anything in particular you think should be removed or utilised.

Have added in as much as I can from the look and feel at the mo, including:

  • fonts (same size for all breakpoints as per design request to see them in situ)
  • CTA's, both inline and standalone
  • buttons
  • colours
  • background colours
  • font colours

There are a couple of bits that I may need to talk to you both about as some may or may not be needed and I don't really know......give me a nudge if there's anything to look at

@@ -1,15 +1,15 @@
// Fonts

Copy link
Contributor

Choose a reason for hiding this comment

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

@wagg-matt @AndyEPhipps Thinking to change a bit here:

$font-light-oblique: "GT-Walsheim-Light-Oblique";
$font-regular: "GT-Walsheim-Regular";
....
$font-stack: helvetica, arial, sans-serif;

$body-font: $font-regular, $font-stack;
$heading-font: $font-black, $font-stack;

Better maintainable. What do you 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 see what you're getting at. Since the fonts may have different weights we could do the following:

$font-light-oblique: "GT-Walsheim-Light-Oblique";
$font-regular: "GT-Walsheim-Regular";
.......
$font-stack: helvetica, arial, sans-serif;
.......

Then do:

font-family: $font-regular $font-stack;

Just because the differing weights requiring a different font will mean we can't just have one for body and one for heading.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, looks good!

Matthew Wagg and others added 3 commits August 5, 2016 15:16
n: 21px,
s: 20px,
xs: 18px,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

new line plz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's on its way up

Copy link
Contributor

Choose a reason for hiding this comment

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

Noice


@mixin font-variation($color) {
.font--#{nth($color, 1)} {
color: nth($color, 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Glad that we're still using these mixins, good stuff

@wagg-matt wagg-matt added this to the 1.7 milestone Aug 10, 2016
@gusliedke gusliedke merged commit 1c152ca into develop Aug 10, 2016
@gusliedke gusliedke deleted the feature/PLAT-360_theme_look_feel branch August 10, 2016 08:41
@Saphyel Saphyel mentioned this pull request Aug 16, 2016
@pvhee pvhee mentioned this pull request Jan 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants