less / less.js

Less. The dynamic stylesheet language.
http://lesscss.org
Apache License 2.0
17.02k stars 3.41k forks source link

Some problems after upgrading to version 4.0 #3578

Closed liby closed 3 years ago

liby commented 3 years ago

Digressions

I'd like to talk about updating the log first:

The last dozen or so version change on less.js are listed in the CHANGELOG.md file. However, the 4.0 changelog is not written here. It's easy to give others the illusion that this update is just an adjustment of the version number.

Looked at the Release page, before the 4.0 version, it has not written the update log for a long time. If I hadn't discovered the problem after upgrading the dependency, I wouldn't have even thought of coming here.

So can you always list the change log in file CHANGELOG.md?

Problem description

After upgrading to version 4.0, the same style is problematic.

Screenshot

3.13 4.0

Related Codes ```html ``` ```css input[type='range'] { width : auto; display : inline-block; } input[type='range'] { @size : 14px; width: 128px; height: @size; padding : 0; // To Fix style on IE @desktop : ~'screen and (min-device-width : 1250px)'; @print : ~'print'; @media @desktop, @print { -webkit-appearance : none; border-radius : 0; position : relative; vertical-align : middle; overflow-x : hidden; overflow-y : visible; background : transparent; outline : none; .range-style { -webkit-appearance : none; height : 2px; background-color : #c1c1c1; border : 0; } &::-webkit-slider-runnable-track { .range-style(); } &::-webkit-slider-thumb { .size(@size, @size); -webkit-appearance : none; pointer-events : none; position : relative; background : #1de9b6; border-radius : 50%; margin-top : -@size / 2 + 1; } .range-thumb { .size(@size, @size); display : block; border-radius : 50%; background : #1de9b6; border : 0; } &::-moz-range-track { .range-style(); } &::-moz-range-thumb { .range-thumb(); } &[readonly], &[disabled], [disabled] & { &::-webkit-slider-thumb { background : #c1c1c1; } &::-moz-range-thumb { background : #c1c1c1; } } &::-ms-fill-lower { height : 2px; background : #1de96b; } &::-ms-track { height : 2px; background : #c1c1c1; color : transparent; border : 0; } &::-ms-thumb { .range-thumb(); // Fix style on Edge margin : 0; } &::-ms-tooltip { display : none; } } } ```
matthew-dean commented 3 years ago

Really these are separate issues, and should be submitted as such. But I'll address what I can here.

  1. The CHANGELOG.md is written automatically from merged commits. I don't have a good / easy way to update that manually and capture all PRs. And it lists up through 4.0, so I'm not sure what you mean by the 4.0 changelog is not there.
  2. Releases will be the place for documenting breaking changes. Not sure of a better way to surface that.
  3. Major version numbers in semver represent breaking changes. So I'm not sure I agree with: "It's easy to give others the illusion that this update is just an adjustment of the version number."
  4. Without seeing what your CSS is compiled as between v3.x and 4.x, I'm not sure what it's doing differently that would lead to that visual issue. Can you compare the compiled output and open a separate issue specific to the difference?
liby commented 3 years ago

Really these are separate issues, and should be submitted as such. But I'll address what I can here.

It's just a digression. image

The CHANGELOG.md is written automatically from merged commits. I don't have a good / easy way to update that manually and capture all PRs. And it lists up through 4.0, so I'm not sure what you mean by the 4.0 changelog is not there.

But you don't list what changes have been made in version 4.0. image

Releases will be the place for documenting breaking changes. Not sure of a better way to surface that.

If you say so, let's look at the Release page, there is no record of any updated content from version 2.7.3 to version 3.0. Is there no breaking changes between them? image

Major version numbers in semver represent breaking changes. So I'm not sure I agree with: "It's easy to give others the illusion that this update is just an adjustment of the version number."

So you should always list the updates of the version in the same place. Like you said the Releases page will list the breaking changes, the CHANGELOG.md simply captures the contents of the PRS to update itself. It's your personal preference and many users don't know this right? Not to mention that, as mentioned above, the Releases page don't always record breaking changes

I personally think it is not appropriate to write the update of version 2.0 in the Releases page and the update of version 3.0 in the CHANGELOG.md.

Like Cypress, it always lists the updates of each version on the Releases page. Like React-Bootstrap, it always lists the updates of each version on the CHANGELOG.md. Like core-js, it lists the changes made in each version on Releases page and in CHANGELOG.md simultaneously.

Without seeing what your CSS is compiled as between v3.x and 4.x, I'm not sure what it's doing differently that would lead to that visual issue. Can you compare the compiled output and open a separate issue specific to the difference?

I will continue to follow up.

matthew-dean commented 3 years ago

@liby I don't necessarily disagree on any particular point, it just needs someone to take it on. If you'd like to take on

  1. updating the changelog manually,
  2. bringing all the releases up-to-date,
  3. documenting which is where,

I'm happy to assist you to get you started!

liby commented 3 years ago

@matthew-dean I'm happy to help! But one of the most important issues at hand is that I can't guarantee that the documentation will be updated in the first place when the version is updated. Or do you already have an idea for a solution?

matthew-dean commented 3 years ago

@liby Because docs aren't in the same repo (they're in less-docs), usually if there's a new feature, then it's generally the responsibility of the contributor to document it / do a PR. However, if it's a bug fix, there's nothing there to do with docs. They always point to the changelog and the repo.

What I meant though is basically if you want to see the changelog documented manually (which it was in the past), then put in place whatever was missing for YOU that you didn't know where to find information on 4.0.

I myself as a Github user am never sure where to find information on new releases. Also, I'm not convinced of your position that it should be in the changelog, because often that makes the changelog too verbose. The changelog should be the briefest of brief summaries across time -- i.e. just the basics, in bullet form of what's different. The releases page should be (IMO) a more detailed description, if applicable.

We don't want to re-invent the wheel. Some of the work of contributing to an OSS project is just finding and adopting "best practices" so that newcomers (like yourself) are not surprised. Less has no corporate sponsors, so every contribution is from someone in the community, improving it a bit at a time.

If you have major changes you propose to improve / organize the repo / codebase, please post to: https://github.com/less/less-meta. (If it's a specific feature for Less, that would still go in these issues. Less-meta is for "organizational" or "larger scope" discussions.)

liby commented 3 years ago

Without seeing what your CSS is compiled as between v3.x and 4.x, I'm not sure what it's doing differently that would lead to that visual issue. Can you compare the compiled output and open a separate issue specific to the difference?

I found the cause of the problem:

@size: 14px;
::-webkit-slider-thumb {
  margin-top: -@size / 2 + 1;
}

This would have been compiled as margin-top: -6px before version 4.0, but in version 4.0 it will be compiled as margin-top: -14px / 2 + 1.

So even though no error is reported here, the division expression should be enclosed in parentheses:

@size: 14px;
::-webkit-slider-thumb {
  margin-top: -(@size / 2) + 1;
}
matthew-dean commented 3 years ago

Ah, I figured it was something like that. I wonder if some kind of console warning can / should be added to root-level division.