dance2die / calendar-dates

📆 Get a list of calendar dates with no external dependencies.
https://www.npmjs.com/package/calendar-dates
MIT License
14 stars 3 forks source link

Unexpected array in getMatrix #20

Closed ewolfe closed 6 years ago

ewolfe commented 6 years ago

Unexpected behavior

All the dates in the last array of getMatrix belong to the next month.

Expected behavior

If all the dates belong to the next month, then the array should probably just not be returned.

Steps to reproduce

https://codesandbox.io/s/10lo3lqn3l

import CalendarDates from "calendar-dates";

const calendarDates = new CalendarDates();

calendarDates.getMatrix(new Date("2018-10-07")).then(dates => {
  console.log(dates);
  document.getElementById("app").innerHTML = `
    <pre>
${JSON.stringify(dates[dates.length - 1], null, 2)}
    </pre>
  `;
});
ewolfe commented 6 years ago

@dance2die I can open a pull request to fix this. We can just check if the 7th item from the last belongs to the next month and then splice it. Does that sound like an okay approach?

dance2die commented 6 years ago

Hi @ewolfe Sorry to get back to ya late.

If next month dates should not be displayed, shouldn't previous month dates also should not include iso date?

ewolfe commented 6 years ago

No, sorry. It's not that all next month dates should not be displayed, it's that too many get displayed. For example, there's no need to display this last row here:

image

dance2die commented 6 years ago

Ah, I see what you mean.

I went with the design decision to be consistent with many of calendars available.

Google, Outlook, & Windows as an example. image

So I'd like to leave it as it is and let users decide whether to filter it out or not.

ewolfe commented 6 years ago

Ah okay, that's fair! Thanks for sharing those screenshots. Will go ahead and close this issue :)