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

Issue with Id Attribute #408

Open
nubranch opened this issue Jan 20, 2021 · 8 comments
Open

Issue with Id Attribute #408

nubranch opened this issue Jan 20, 2021 · 8 comments

Comments

@nubranch
Copy link

I'm testing MinimalCSS on a site that's using Id attribute in the main navigation. MinimalCSS is not including the corresponding css. This is a WordPress theme.

HTML: mainnav attribute used in nav below:

<nav id="mainnav" class="mainnav" role="navigation">
	<ul id="menu-main" class="menu">
		<li id="menu-item-3266" class="menu-item">
			<a href="https://www.smcgroup.io/" aria-current="page">Home</a>
		</li>

Original CSS: (this is all missing from final CSS)

#mainnav ul {
    list-style: none;
    margin: 0;
    padding: 0;
}

#mainnav>ul>li {
    display: inline-block;
}

#mainnav ul li {
    position: relative;
}

.nav.header-style4 #mainnav>ul>li>a {
    color: #fff;
    line-height: 65px;
}

#mainnav>ul>li>a {
    position: relative;
    display: block;
    font-family: "Montserrat",sans-serif;
    text-transform: uppercase;
    color: #303030;
    text-decoration: none;
    font-size: 14px;
    margin: 0 16px;
    -webkit-transition: all .3s ease-in-out;
    -moz-transition: all .3s ease-in-out;
    -ms-transition: all .3s ease-in-out;
    -o-transition: all .3s ease-in-out;
    transition: all .3s ease-in-out;
}


@peterbe
Copy link
Owner

peterbe commented Jan 20, 2021

Do you have a URL I can test this against?
Or better can you clone minimalcss locally and debug why it's claiming certain selectors are not present in the DOM.

@nubranch
Copy link
Author

Hi Peter. Here's a sample URL:
https://www.smcgroup.io/

@peterbe
Copy link
Owner

peterbe commented Jan 20, 2021

▶ ./bin/minimalcss.js https://www.smcgroup.io/
Error: Invalid CSS found while evaluating https://www.smcgroup.io/wp-content/litespeed/cssjs/b3856.css?50a30: ".flat-top. ul.nah-socials li"
    at Object.enter (/Users/peterbe/dev/JAVASCRIPT/minimalcss/src/run.js:563:19)
    at Object.<anonymous> (/Users/peterbe/dev/JAVASCRIPT/minimalcss/node_modules/css-tree/lib/walker/create.js:11:16)
    at List.walkNode (/Users/peterbe/dev/JAVASCRIPT/minimalcss/node_modules/css-tree/lib/walker/create.js:161:19)
    at List.each (/Users/peterbe/dev/JAVASCRIPT/minimalcss/node_modules/css-tree/lib/common/List.js:158:12)
    at Object.StyleSheet (/Users/peterbe/dev/JAVASCRIPT/minimalcss/node_modules/css-tree/lib/walker/create.js:106:29)
    at walkNode (/Users/peterbe/dev/JAVASCRIPT/minimalcss/node_modules/css-tree/lib/walker/create.js:164:37)
    at Object.walk (/Users/peterbe/dev/JAVASCRIPT/minimalcss/node_modules/css-tree/lib/walker/create.js:220:9)
    at /Users/peterbe/dev/JAVASCRIPT/minimalcss/src/run.js:546:13
    at Array.forEach (<anonymous>)
    at Object.minimalcss [as run] (/Users/peterbe/dev/JAVASCRIPT/minimalcss/src/run.js:532:12)

You can see the error here too: https://minimalcss.app/?url=https%3A%2F%2Fwww.smcgroup.io%2F

@nubranch
Copy link
Author

I find that quite a few themes have css errors, so I've enabled ignoreCSSErrors to avoid not getting the css output.

@peterbe
Copy link
Owner

peterbe commented Jan 20, 2021

I tried this:

const prettier = require('prettier');
const minimalcss = require('.');
minimalcss
  .minimize({
    url: 'https://www.smcgroup.io/',
    ignoreCSSErrors: true,
  })
  .then((result) => {
    const codeBefore = result.finalCss;
    // console.log(codeBefore);
    codeAfter = prettier.format(codeBefore, {
      semi: true,
      parser: 'css',
    });
    console.log(codeAfter);
  });

And if I inspect the CSS that prints out, I do see:

#mainnav ul li,
#mainnav > ul > li > a,
.header,
.nav-wrap,
.wrap-header-style4 {
  position: relative;
}

that's one of the selectors you claimed didn't make it in the minimal CSS.

@nubranch
Copy link
Author

Thanks for sharing the code.

I have been using the js code that was part of the npm install, which includes the following options:

const options = {
  urls: urls,
  debug: argv['debug'],
  loadimages: argv['loadimages'],
  styletags: argv['styletags'],
  withoutjavascript: argv['withoutjavascript'],
  skippable: (request) => {
    let skips = argv['skip'];
    if (!skips) {
      return false;
    }
    if (!Array.isArray(skips)) {
      skips = [skips];
    }
    return skips.some((skip) => !!request.url().match(skip));
  },
  viewport: parseViewport(argv['viewport']),
  puppeteerArgs: argv['nosandbox']
    ? ['--no-sandbox', '--disable-setuid-sandbox']
    : [],
  enableServiceWorkers: argv['enableserviceworkers'],
};

Since I wasn't passing a value for "withoutjavascript", it seems the the value was being set to false. If I set the value of "withoutjavascript" to true (or comment that line out), then the final css does include the missing css selectors.

I've used the default options above for a bunch of websites and I didn't have issues. I will need to re-test those websites again to see if omitting the flag will make a difference.

thanks.

@peterbe
Copy link
Owner

peterbe commented Jan 21, 2021

Maybe I don't exactly understand your point. (But I get that it works now when you do a render without JS).
If the README is wrong/misleading, would you mind contributing a quick PR to update it so the "next you" don't fall into the same trap?

@nubranch
Copy link
Author

I think it's more like me not understanding that not passing a value through the command line and having the code expecting the parameters (through argv statement) results in the parameter being set to false.

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

No branches or pull requests

2 participants