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

Gantt test broken #4198

Closed
sidharthv96 opened this issue Mar 7, 2023 · 3 comments · Fixed by #4207
Closed

Gantt test broken #4198

sidharthv96 opened this issue Mar 7, 2023 · 3 comments · Fixed by #4207
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect

Comments

@sidharthv96
Copy link
Member

Description

In 9.4.2

image

In 10

image

demo says line should be invisible, but it's visible.

Steps to reproduce

gantt
    title Hide today marker (vertical line should not be visible)
    dateFormat Z
    axisFormat %d/%m
    todayMarker off
    section Section1
    Today: 1, -01:00, 5min
Loading

Screenshots

No response

Code Sample

No response

Setup

No response

Additional Context

No response

@sidharthv96 sidharthv96 added Type: Bug / Error Something isn't working or is incorrect Status: Triage Needs to be verified, categorized, etc labels Mar 7, 2023
@sidharthv96
Copy link
Member Author

@aloisklink possibly related to dayjs? But not sure why the behaviour is different between v9 & v10

@aloisklink
Copy link
Member

I just ran Cypress on commit 06640ab (aka the one right before a5db04b, where we added dayjs), and I'm getting the same diagram, so dayjs didn't change anything.

Cropped from cypress/snapshots/rendering/gantt.spec.js/Gantt-diagram-should-hide-today-marker.snap.png in commit 06640ab:

image

The only e2e test snapshot that changed in the switch to dayjs for me was the test I mentioned in #4153 (comment), however, the demo line for that test says: "should FAIL..." but it clearly works, so 🤷

By the way, we should probably make something like a develop-v9 branch (if we ever do make any more v9 releases), instead of committing duplicate commits to master/develop. Otherwise it makes the git history a bit confusing :)

The OpenWRT repo is a good example, since they are always committing backports: https://github.com/openwrt/openwrt

@aloisklink
Copy link
Member

Hmmmm, apparently the today marker isn't there. Even when I changed todayMarker on, it doesn't seem to appear.

See below for an actual working test:

```mermaid
gantt
    title todayMarker on
    dateFormat YYYY-MM-DD
    axisFormat %d
    todayMarker on
    section Section1
        Today: 2023-03-08, -1h
        The future: 2025-01-01
```
gantt
    title todayMarker on
    dateFormat YYYY-MM-DD
    axisFormat %d
    todayMarker on
    section Section1
        Today: 2023-03-08, -1h
        The future: 2025-01-01
Loading

And the same diagram with todayMarker off:

gantt
    title todayMarker off
    dateFormat YYYY-MM-DD
    axisFormat %d
    todayMarker off
    section Section1
        Today: 2023-03-08, -1h
        The future: 2025-01-01
Loading

aloisklink added a commit to aloisklink/mermaid that referenced this issue Mar 9, 2023
The gantt diagram that were supposed to test whether
`todayMarker off` works wasn't working properly, because
`todayMarker on` wasn't working (i.e. the test never failed).

I've fixed this issue, and added a test that checks whether
`todayMarker on` works.

Fixes: mermaid-js#4198
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants