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

Block selection of end date earlier from start date #309

Closed
yardenmezi opened this issue Dec 16, 2023 · 12 comments · Fixed by #521
Closed

Block selection of end date earlier from start date #309

yardenmezi opened this issue Dec 16, 2023 · 12 comments · Fixed by #521
Assignees

Comments

@yardenmezi
Copy link
Collaborator

In pages that have date range selection, the End date shouldn't allowed to be before the start date.
The current pages are dashboard and gaps_patterns.

@amabelleS
Copy link
Collaborator

I'll assign it to me:)

Should I tag the commits as a refactor or fix?

@amabelleS amabelleS self-assigned this Dec 30, 2023
@amabelleS
Copy link
Collaborator

amabelleS commented Dec 30, 2023

Maybe we also want to show an error message or alert the user that he/she entered an end date before the start date?
I think this will be ok:
image

I also added it to the translations module:
"bug_date_alert": "נא וודא שתאריך הסיום מאוחר יותר מתהאריך ההתחלה",
"bug_date_alert": "Please make sure that the end date is later than the start date",

@amabelleS
Copy link
Collaborator

I think this issue was fixed. I made a PR for it some time ago...
Is there still some issue with the dates? Or can we close this one?

@NoamGaash @shootermv

@NoamGaash
Copy link
Member

not needed - if it's fixed, let's close that issue. thanks!

@NoamGaash
Copy link
Member

Wait - it's not blocked. It only gives a warning.
This issue is still relevant
image

@NoamGaash NoamGaash reopened this Feb 10, 2024
@amabelleS
Copy link
Collaborator

Wait - it's not blocked. It only gives a warning. This issue is still relevant

צודק. אני חושבת שחיפשתי אז דרך להגביר עם פונקציה שכתבתי, וראיתי בפועל שזה משנה את התאריך בGUI אבל בפועל לא נותן תוצאות מהשרת ככה. אז חשבתי שרק התרעה תספיק.
מצאתי עכשיו פתרון יותר טוב אולי. אפשר לשים minDate, אז ערכתי את הDatePicker ככה שלא יוכל לקבל ערכים קטנים מתאריך ההתחלה. השאלה אם יש עוד עמודים שצריך ליישם בהם את זה חוץ מהדאשבורד והgaps_patterns ? אולי כבר משהו השתנה מאז... שלא אפספס.

@Haswell-s
Copy link
Collaborator

Haswell-s commented Feb 24, 2024

היי @amabelleS , זה בדיוק מה שאני עשיתי, יש בחירה מהסוג הזה בדף הראשי ובדפוסי נסיעות.
חשבתי שאת עבר לא עובדת על האישיו אז יש לי pr מוכן, אם תרצי להמשיך לעבוד עליו אני יכול להעביר לך את השינויים.
ובאופן כללי לדעתי הפתרון הכי טוב זה ליצור קומפוננטה חדשה שמורכבת מ2 DatePicker שתקרא DateRangePicker (כמו בגרסאת הפרו)
@NoamGaash מה דעתך?

@amabelleS
Copy link
Collaborator

amabelleS commented Feb 24, 2024

@i5x64BIT
מה מס' ה-PR שפתחת עליו?
האמת כתבתי את ההודעה כמה דק לפני שפתחתי עליו PR. #521
ממליצה לכתוב פה הודעה אם אתה עובד על אישיו, ככה לא יהיו כפילויות ועבודה סתם.. יהיה אפשר לשייך אותו אליך.

לא יודעת מה עדיף, שני datePickers או אחד. לא הבנתי למה שניים.. כרגע היינו צריכים לממש min אז הוספתי לקיים, אבל כפרופ אופציונאלי. אפשר להוסיף גם maxDate כפרופ' אופציונאלי, השאלה אם יש בכך צורך כרגע. @NoamGaash ?

@Haswell-s
Copy link
Collaborator

Haswell-s commented Feb 24, 2024

@amabelleS

2 DatePicker יחד
אפשר גם להוסיף לו עוד לוגיקה במידת הצורך.
הבעיה עם הפתרון של minTime maxTime זה הבילבול

@NoamGaash
Copy link
Member

קומפוננטה לrange נשמע רעיון מעולה.
אם תרצו אפשר לעשות סקר קוד זה לזה, שנכניס גרסה משותפת :)

@Haswell-s
Copy link
Collaborator

קומפוננטה לrange נשמע רעיון מעולה.
אם תרצו אפשר לעשות סקר קוד זה לזה, שנכניס גרסה משותפת :)

@NoamGaash האמת שאין לי מושג מה זה

@NoamGaash
Copy link
Member

code review - https://github.com/features/code-review
בשביל להכניס לכאן קוד, לא צריך אישור שלי - מספיק שמישהו עושה PR ומישהו נוסף מאשר לו, והטסטים עוברים, ואפשר למזג

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 a pull request may close this issue.

4 participants