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

Replace drop shadow around .sul-embed-container with a simple border. #610

Merged
merged 2 commits into from
Jul 18, 2016

Conversation

tingulfsen
Copy link
Contributor

Fixes #604

Before:

before

After:

after

I've just verified that this seems to work fine with #609

@coveralls
Copy link

coveralls commented Jul 15, 2016

Coverage Status

Coverage remained the same at 98.46% when pulling fe991d5 on 604-drop-shadow-border into 4511780 on master.

@@ -13,7 +13,7 @@
font-family: 'Source Sans Pro', 'Arial Unicode MS', Helvetica, sans-serif;
font-size: $font-size-base;
overflow-y: hidden;
@include shadow(3);
border: 1px solid #5f574F;
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't reference colors directly in our css declarations. This is a color that is defined in SUL Styles as $color-pantone-405 (which we have defined as $sul-border-color in SUL Embed). Can we use that variable here instead if the color itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I do see that this was used below (although I don't think in a border context). We should probably fix that too, but don't feel obligated to include that in this PR.

@jmartin-sul
Copy link
Member

worked for me on my laptop.

since tommy's on vacation and jessie's suggestion seemed both reasonable and straight-forward, i pushed a commit fixing both of the explicit color references. still works on my laptop.

@coveralls
Copy link

coveralls commented Jul 18, 2016

Coverage Status

Coverage remained the same at 98.46% when pulling e4e44b0 on 604-drop-shadow-border into 4511780 on master.

@ndushay ndushay merged commit 467103f into master Jul 18, 2016
@ndushay ndushay deleted the 604-drop-shadow-border branch July 18, 2016 19:08
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.

5 participants