Navigation Menu

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

Support media attribute #506

Conversation

bookwyrm
Copy link

@bookwyrm bookwyrm commented Jun 25, 2021

Adds support to wrap CSS with media query if the original link has a media attribute.

Fixes #443

So that we can extract them when we build the CSS later
Need to check for presence of media and extract manually (rather than use `urlObj`) to support local filesystem references.
@bookwyrm bookwyrm changed the title Support media attribute - fixes #443 Support media attribute Jun 25, 2021
Copy link
Collaborator

@bezoerb bezoerb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR but i think the media queries should be handled should be handled differently. I'll try to draft something myself and i might take some lines from this PR :)

@@ -391,6 +393,14 @@ function getStylesheetHrefs(file) {
return Buffer.from(link.value);
}

if (hasMeaningfulMedia(link.$el)) {
if (link.value.includes('?')) {
link.value += `&media=${encodeURI(link.$el.attr('media'))}`;
Copy link
Collaborator

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 should append query params to the stylesheet links. This could lead to errors as we don't always know how these are processed by the server

@bezoerb
Copy link
Collaborator

bezoerb commented Jul 19, 2021

@bookwyrm: Thanks again for this PR. It helped me kickstart #510 where i took a slightly different approach to store the media attribute.

@bezoerb bezoerb closed this Jul 19, 2021
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.

Media query from <link> should be used to wrap the <link> contents
2 participants