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

Implement Validation for Date Picker #2165

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

JeevaRamanathan
Copy link
Contributor

Hi @FredLL-Avaiga,
Referencing issue #848, I’ve added the following specifications for date validation to handle date disabling options:

{
    disableWeekdays?: boolean;
    disableWeekends?: boolean;
    disablePastDays?: boolean;
    disableFutureDays?: boolean;
    dayOfWeek?: number; // 0-6 (0 = Monday, ..., 6 = Sunday)
    interval?: number;
    oddInterval?:boolean;
    occurence?: number;
}

The rules for disabling dates include options for disabling past days or future days, weekdays, weekends, or specific day of the month with disablePastDays, disableFutureDays,disableWeekdays, disableWeekends,dayOfWeek.

To disable alternating days:

disable_date_config_rule = {
    dayOfWeek: 3,
    interval: 2 // Disables alternate Wednesdays by default (even weeks)
}
<|{date}|date|disable_date_config={disable_date_config_rule}|>

Setting oddInterval: True will disable Wednesdays on the 1st, 3rd, and 5th weeks

To disable the nth occurrence of a specific weekday in the month:

disable_date_config_rule = {
    dayOfWeek: 1,
    occurrence: 1 // Disables the first Monday of each month
}

Could you please review this and let me know if any modifications are needed for better handling of these specifications?

Signed-off-by: JeevaRamanathan M <[email protected]>
@FredLL-Avaiga
Copy link
Member

I think a dict (object) might not be the more user-friendly way for the user...
Let's call a friend : @FabienLelaquais what do you think?

@FabienLelaquais
Copy link
Member

FabienLelaquais commented Oct 26, 2024

I think a dict (object) might not be the more user-friendly way for the user... Let's call a friend : @FabienLelaquais what do you think?

Actually I do think that a dict may be the only option we have.
The expressivity we want is going to need several slots that are specified or not.

However, I wouldn't hardcode the notion of weekend or workday. This varies across cultures.

My first thoughts about this would be a dict with two optional slots: one would indicate dates that must be forbidden, another would be dates that are explicitly available.
Say you want to book a table at a restaurant that opens only on Thursdays and Fridays, but closes for the Christmas season.
So you want to allow the selection of all the future Thursdays and Fridays to be selectable, except for the vacation period.

The way I see you can express this is:

date_constraints = {
   "accept": {
       "days_of_week": [3, 4]
     },
     "reject": {
       "past": True,
       "period": [ [ (2024, 12, 23), (2024, 12, 31) ] ]
      }
}

This has the issue of having days that fall in the two buckets, the a priority may be needed.

Now I like the 'occurrence' thing but we need to allow for more expressivity, again.
Like catching "U.S. presidential elections are held every four years on the first Tuesday after the first Monday in November".
That sounds tricky to me... except if you allow for an explicit list of accepted dates.

Same for 'repeat' (or 'interval'). The period (week, month, year...) needs to be expressed somehow.

@JeevaRamanathan
Copy link
Contributor Author

This has the issue of having days that fall in the two buckets, the a priority may be needed.

@FabienLelaquais, for example, Wednesday and Thursday (days 3 and 4) are accepted but also fall within the rejection period. Are you referring to this as falling into two buckets and needing prioritization, or is it something different?

@JeevaRamanathan
Copy link
Contributor Author

JeevaRamanathan commented Oct 28, 2024

Hi @FabienLelaquais, I'd like to get your thoughts on the updated structure for date_constraints. I’ve added support for priority, weekly, monthly, yearly rules along with the after week attribute to address the cases like US election logic.

  1. Weekly
  const date_constraints = {
    priority: "reject", //optional
    accept: {
      day_of_week: [0, 1],
    },
    reject: {
      past: true,
      period: [
        { start: "2024, 11, 23", end: "2024, 11, 30 " },
        { start: "2025, 11, 23", end: "2025, 11, 30" },
      ],
      repeat: {
        frequency: "weekly",
        occurrence: [
          {
            day_of_week: 1, // disables every monday
            interval: 1,
            odd_interval: false,
          },
          {
            day_of_week: 3,  // disables alternate wednesday
            interval: 2,
            odd_interval: false,
          },
        ],
      },
    },
  };
  1. Monthly
const date_constraints = {
    priority: "reject",
    accept: {
      // day_of_week: [0, 1],
    },
    reject: {
      future:true,
      repeat: {
        frequency: "monthly",
        occurrence: [
          {
            day_of_week: 6, // saturday
            after: {
              week: 1, //first week is skipped
            },
            interval: 2, // specific interval for this occurrence
          },
          {
            day_of_week: 5,
            after: {
              week: 3,
            },
            interval: 3,
          },
        ],
      },
    },
  };
  1. Yearly - This handles the yearly constraints for specific days, like the US election, where the date pattern is every 4 years on the first Tuesday after the first Monday in November using the after week property.
const date_constraints = {
    accept: {
      // day_of_week: [0, 1]
    },
    reject: {
      past: true,
      // future: false,
      period: [],
      repeat: {
        frequency: "yearly",
        occurrence: [
          {
            month: 10, // November
            day_of_week: 2, // Tuesday
            after: {
              week: 1, // After the first week
            },
            interval: 4, // Every 4 years
            start_year: 2024, // Start from the year 2024
          },
          {
            month: 6, // July 
            day_of_week: 5, // Friday
            after: {
              week: 2// After the second week
            },
            interval: 3, // Every 3 years
            start_year: 2025, // Start from the year 2025
          },
        ],
      },
    },
  };

Prolly the same repeat format can be used for accept; Here's the sample frontend workaround logic for this structure.

Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you fix the failing checks ?

taipy/gui/_renderers/factory.py Outdated Show resolved Hide resolved
Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see a few frontend tests.
Can you add a time constraint in the rules ? (ie for example allow only to select time that are multiple of 15 minutes)

analogic? :boolean;
analogic?: boolean;
disableDateConfig?: string;
defaultDisableDateConfig?: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for this as it is not dynamic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to confirm the structure. Can I make these changes to the initial structure for the time?

{
disableWeekdays?: boolean;
disableWeekends?: boolean;
disablePastDays?: boolean;
disableFutureDays?: boolean;
dayOfWeek?: number; // 0-6 (0 = Monday, ..., 6 = Sunday)
interval?: number;
oddInterval?:boolean;
occurence?: number;
}

Or should I follow the new logic that is currently under discussion and has not been finalized after FabienLelaquais comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll wait for @FabienLelaquais 's input

@@ -74,6 +86,8 @@ const DateSelector = (props: DateSelectorProps) => {
const hover = useDynamicProperty(props.hoverText, props.defaultHoverText, undefined);
const min = useDynamicProperty(props.min, props.defaultMin, undefined);
const max = useDynamicProperty(props.max, props.defaultMax, undefined);
const emptyDateConfig = {} as Partial<DisableDateConfig>;
const disableDateConfig = useDynamicJsonProperty(props.disableDateConfig, props.defaultDisableDateConfig || "", emptyDateConfig);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for this as it is not dynamic, but your need to JSON parse it

@@ -115,25 +129,76 @@ const DateSelector = (props: DateSelectorProps) => {
}
}, [props.date, tz, withTime, max, min]);


const getWeekNumberInMonth = (date: Date) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be defined outside the component

const weekNo: number = Math.ceil(((d.getTime() - firstDayOfMonth.getTime()) / 86400000 + 1) / 7);
return weekNo;
}
const isDisabledDate = (date: Date) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useCallback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants