-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
CSS linting: Adding stylelint-config-standard #6071
Conversation
dist/mapbox-gl.css
Outdated
background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg%20viewBox%3D%270%200%2020%2020%27%20xmlns%3D%27http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%27%3E%0D%0A%09%3Cpath%20fill%3D%27%23333333%27%20fill-rule%3D%27evenodd%27%20d%3D%27M4%2C10a6%2C6%200%201%2C0%2012%2C0a6%2C6%200%201%2C0%20-12%2C0%20M9%2C7a1%2C1%200%201%2C0%202%2C0a1%2C1%200%201%2C0%20-2%2C0%20M9%2C10a1%2C1%200%201%2C1%202%2C0l0%2C3a1%2C1%200%201%2C1%20-2%2C0%27%20%2F%3E%0D%0A%3C%2Fsvg%3E"); | ||
background-color: rgba(255, 255, 255, .5); | ||
width: 24px; | ||
h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the idea of mixing these all together, since they are unrelated, and just happen to have the same values.
It also mixes up the stylesheet document, instead of having attribution styling together and then user-location-dot styling together it's now mixed.
26cae97
to
fc6d62e
Compare
@andrewharvey Makes sense — I've reverted that change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasonbarry thanks for the contribution!
we use 4 spaces for indentation in mapbox-gl and stylelint-config-standard uses 2, so we should override that rule if we do decide to move forward with this new dependency.
Fixes the .mapbox-improve-map element to be .mapboxgl-improve-map (uses correct prefix)
mapbox-improve-map
is ignored because the HTML for the attribution comes as part of the Mapbox Streets TileJSON, so please revert the changes to that class name.
.mapboxgl-ctrl-attrib a:hover { | ||
color: inherit; | ||
text-decoration: underline; | ||
} | ||
/* stylelint-disable */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still need to disable stylelint here and revert the mapboxgl-improve-map
–> mapbox-improve-map
per my previous comment
.stylelintrc
Outdated
"rules": { | ||
"selector-class-pattern": "mapboxgl-[a-z-]+" | ||
"indentation": 4, | ||
"selector-class-pattern": "mapbox(gl)?-[a-z-]+" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mollymerp I've done that, just in a different location: the .stylelintrc file now has a more flexible selector pattern so that those disable comments aren't needed in the CSS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to not loosen this requirement for the entire codebase, and keep the disable rule inline.
dist/mapbox-gl.css
Outdated
/* stylelint-disable */ | ||
.mapboxgl-ctrl-attrib .mapbox-improve-map { | ||
|
||
.mapboxgl-ctrl-attrib .mapboxgl-improve-map { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be changed back to mapbox-improve-map
for the CSS rule to apply to the attribution control
thanks again for the contribution @jasonbarry ! |
* Adding stylelint-config-standard * Passing linting * border: none -> border: 0 * Addressing feedback * Fixing last instance of incorrect class name * mapbox-ctrl-bottom-right -> mapboxgl-ctrl-bottom-right * Stricter selector pattern
* Adding stylelint-config-standard * Passing linting * border: none -> border: 0 * Addressing feedback * Fixing last instance of incorrect class name * mapbox-ctrl-bottom-right -> mapboxgl-ctrl-bottom-right * Stricter selector pattern
This project already uses
stylelint
to enforce that CSS classes are prefixed withmapboxgl-
. However, no other rules are enforced.This PR configures the CSS to adhere to the
stylelint-config-standard
configuration, a common set of rules for stricter CSS linting. The result is a neaterdist/mapbox-gl.css
file.yarn lint-css
results in 0 errors