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.

Sign up for GitHub

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

Jump to bottom

[date-picker] Month duplication caused by font size #1665

Closed
2 tasks done
aszalai opened this issue May 7, 2020 · 9 comments · Fixed by #8278
Closed
2 tasks done

[date-picker] Month duplication caused by font size #1665

aszalai opened this issue May 7, 2020 · 9 comments · Fixed by #8278
Assignees
@web-padawan
Labels
BFP Bug fix prioritised by a customer bug Something isn't working Impact: Low Severity: Major vaadin-date-picker

Comments

@aszalai
Copy link

aszalai commented May 7, 2020

Description

I have a DatePicker and when I scroll down in it the same month appears twice within the same year. The duplicated month is the 3rd and 4th after the initial. The error occures, when the --lumo-font-size-xs CSS property is set to 0.982rem or smaller (0.983rem works fine).

Expected outcome

Every month appears once within the same year.

Actual outcome

The 3rd and 4th month after the initial is the same (duplicated) within the same year.
See example video below.

Example

datepicker-bug

CSS:

html {
	--lumo-font-size-xs: 0.975rem !important;
}

Java code:

import com.vaadin.flow.component.datepicker.DatePicker;
import com.vaadin.flow.component.dependency.CssImport;
import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.router.Route;

@Route("")
@CssImport("./styles/shared-styles.css")
public class MainView extends VerticalLayout {

	public MainView() {
		add(new DatePicker());
	}
}

Vaadin version

Both 14.1.4 and 14.1.27.

Browsers Affected

  • Chrome
  • Firefox
@Haprog
Copy link
Contributor

Haprog commented May 12, 2020

Very weird, but I can confirm. It can be reproduced at https://cdn.vaadin.com/vaadin-date-picker/4.1.1/demo/#date-picker-basic-demos by setting that style on the <html> element (before opening the date picker for the first time).

e.g.

document.documentElement.style.setProperty('--lumo-font-size-xs', '0.975rem');

I also noticed that both August and October are duplicated (on this year when testing now), but if you scroll down past them and then when you scroll back up there is only one of them. And again if you scroll up past them a few months and then back down, then they are again duplicated.

If you select a value in those months and then open the picker again, then they don't seem duplicated.

@vaadin-bot vaadin-bot transferred this issue from vaadin/vaadin-date-picker May 19, 2021
@web-padawan web-padawan changed the title DatePicker month duplication caused by font size [date-picker] Month duplication caused by font size May 21, 2021
@igormilina
Copy link

I can confirm that month duplication is still present in version 14.4.10. It can be easy reproduced just by opening:
https://vaadin.com/components/vaadin-date-picker/java-examples
in your browser and changing font-size to 15px in html tag (using developer tools). After setting font size, you need to click to open some other component, and then go back to date picker (this will refresh date picker page, but with changed font size)

Or, by setting font size in application code:
html {
font-size: 15px;
}

Font size of 16px works ok, but font size 15px duplicates "January".

@igormilina
Copy link

I've noticed that month duplication is still present in version 23. On https://vaadin.com/components/vaadin-date-picker/java-examples page I got "May" and "July" duplicated by setting font size to 15px.

@maskedmonkey
Copy link

I've noticed the same issue with Jan 2024 being displayed twice. (Vaadin 22)
Screenshot 2024-01-30 at 12 06 16 PM

@TatuLund
Copy link
Contributor

TatuLund commented Dec 5, 2024

When using the following CSS the issue is reproducible in Vaadin 23.5, but not in Vaadin 24.5.

html {
    --lumo-font-size-m: 0.875rem;
    --lumo-space-m: 0.3rem;
}

@web-padawan web-padawan added the BFP Bug fix prioritised by a customer label Dec 5, 2024
@web-padawan
Copy link
Member

The issue can still be reproduced also in 24.6 (main branch) by using font-size: 15px as posted above:

Screenshot 2024-12-05 at 10 37 54

@web-padawan
Copy link
Member

web-padawan commented Dec 5, 2024 •
edited
Loading

The issue appears to be caused by incorrect initial index calculated in this line:

web-components/packages/date-picker/src/vaadin-infinite-scroller.js

Line 394 in 4f7958c

this._firstIndex = ~~((this._buffers[0].translateY - this._initialScroll) / this.itemHeight) + this._initialIndex;

By default it returns -3 for vaadin-month-calendar. However, with changed font-size etc the division inside might be -2.999 and then it's converted to -2 due to usage of ~~ which behaves differently for negative values:

It is used as a faster substitute for Math.floor() for positive numbers. It does not return the same result as Math.floor() for negative numbers, as it just chops off the part after the decimal (see other answers for examples of this).

IMO we should use Math.floor() explicitly since it's basically a micro-optimization:

With optimisation of the JavaScript engine in browsers, the performance for operators and functions change. With current browsers, using ~~ instead of Math.floor is somewhat faster in some browsers, and not faster at all in some browsers. If you really need that extra bit of performance, you would need to write different optimised code for each browser.

UPD: changing it doesn't entirely fix the problem for me since duplicate "March 2025" goes away but there's also duplicate "May 2025" which is caused by 2.99 instead of 3 - so we actually need Math.round() there.

@web-padawan web-padawan self-assigned this Dec 5, 2024
@web-padawan web-padawan mentioned this issue Dec 5, 2024
Merged
@vursen
Copy link
Contributor

vursen commented Dec 5, 2024 •
edited
Loading

UPD: changing it doesn't entirely fix the problem for me since duplicate "March 2025" goes away but there's also duplicate "May 2025" which is caused by 2.99 instead of 3 - so we actually need Math.round() there.

Could there be an error in the calculation caused by conversion of em to px?

web-components/packages/date-picker/src/vaadin-infinite-scroller.js

Lines 134 to 138 in 5eaff4d

const tmpStyleProp = 'background-position';
this.$.fullHeight.style.setProperty(tmpStyleProp, itemHeight);
const itemHeightPx = getComputedStyle(this.$.fullHeight).getPropertyValue(tmpStyleProp);
this.$.fullHeight.style.removeProperty(tmpStyleProp);
this._itemHeightVal = parseFloat(itemHeightPx);

@web-padawan
Copy link
Member

The same can be generally achieved also with fractional px as shown in the test that I added. And also I do think that we need to use Math.round() so that we get -3 for -2.99 and 3 for 2.99 - otherwise buffers are messed up (basically, the item with same index ends up being present in both buffers).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@web-padawan web-padawan

Labels
BFP Bug fix prioritised by a customer bug Something isn't working Impact: Low Severity: Major vaadin-date-picker
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: use Math.round to get correct scroller first index vaadin/web-components
8 participants
@Haprog @vursen @web-padawan @TatuLund @vaadin-bot @aszalai @igormilina @maskedmonkey