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

added a new prop for simple calendar selection #7055 #7135

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sabarnix
Copy link
Contributor

What does this PR do?

Current calendar range selection is ambiguous without the start date and end date inputs. Added a prop simpleSelection which is more predictive selection of date.

Where should the reviewer start?

What testing has been done on this PR?

test cases are pending

How should this be manually tested?

Visualizations/Calendar/RangeSimple
this storybook has the working example

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

#7055

Screenshots (if appropriate)

Do the grommet docs need to be updated?

yes

Should this PR be mentioned in the release notes?

Is this change backwards compatible or is it a breaking change?

yes

@jcfilben jcfilben added the PRty Biweekly PR reviews by grommet team with hoping of closing such PRs label Feb 15, 2024
@britt6612
Copy link
Collaborator

@sabarnix thank you for starting this PR just want to let you know that we are going to discuss this behavior a little more among our team.. Here is another issue as well that we have been dropping some comments in regarding this same behavior.

@britt6612 britt6612 added the discussion Needs deeper discussions label Feb 15, 2024
@jcfilben jcfilben removed the PRty Biweekly PR reviews by grommet team with hoping of closing such PRs label Feb 15, 2024
@sabarnix
Copy link
Contributor Author

sabarnix commented Feb 20, 2024

@britt6612 I was able to achieve the functionality by using activeDate prop.
https://codesandbox.io/p/sandbox/quizzical-cannon-qzw7sl?file=%2Fsrc%2FApp.js%3A17%2C41
This breaks on selecting same date as start and end. Is there a possible solution to this?

@jcfilben jcfilben added the PRty Biweekly PR reviews by grommet team with hoping of closing such PRs label Feb 29, 2024
@britt6612
Copy link
Collaborator

@sabarnix no solution here would you mind filing an issue?

@sabarnix
Copy link
Contributor Author

sabarnix commented Mar 1, 2024

@britt6612 sure

@jcfilben jcfilben removed the PRty Biweekly PR reviews by grommet team with hoping of closing such PRs label Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Needs deeper discussions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants