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

Ashanti/optimize print styles #236

Merged
merged 25 commits into from
Feb 26, 2020
Merged

Conversation

AshantiCode
Copy link
Contributor

I did this without @media only screen, unfortunately i lost my notes about it. But this might be a good global approach.

@AshantiCode AshantiCode self-assigned this Feb 18, 2020
@@ -4,3 +4,5 @@

// stylelint-disable-next-line scss/at-import-no-partial-leading-underscore, scss/at-import-partial-extension-blacklist
@import '../Components/**/_admin.scss';

@import 'styles/print';
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to have the print styles in the admin sheet.

[is='flynt-navigation-footer'],
[is="flynt-block-cookie-notice"],
.button,
footer {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of hiding individual components, I would suggest to hide the wrapping sections of the layout: .mainHeader, .mainFooter, .wpFooter.
Also, I think buttons should still be visible and not every footer element should be hidden, as it might be used differently in a context where you want it to be visible.

display: none !important;
}

a {
Copy link
Member

Choose a reason for hiding this comment

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

I noticed this one is not having the :visited state that the HTML5 Boilerplate suggests. Why?

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 thought that this rule for the tag will apply to every tag on the page, if a:visited will get underlined, it will anyways get underlined, cause it is an tag. But obviously my thoughts were wrong due to your question.


a {
text-decoration: underline;
font-weight: bold;
Copy link
Member

Choose a reason for hiding this comment

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

Why make links bold? I believe they should look like in the design, but black and underlined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay,removed it.

content: " (" attr(href) ")";
}

a[href^="/"]:after {
Copy link
Member

Choose a reason for hiding this comment

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

I assume this would not show the urls of internal links, right? Why should we hide those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was something I found when searching for print styles on the web. I removed it.

@steffenbew steffenbew self-assigned this Feb 21, 2020
@AshantiCode
Copy link
Contributor Author

I added some rules to the .component-footer of ListComponents otherwise the link addresses weren't readable and would overlap.

@steffenbew steffenbew merged commit fc37b44 into master Feb 26, 2020
@steffenbew steffenbew deleted the ashanti/OptimizePrintStyles branch February 26, 2020 11:19
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.

2 participants