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

feat: enable date range state to be controlled #1198

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

Conversation

brennarvo
Copy link
Collaborator

@brennarvo brennarvo commented Apr 7, 2022

Please check if the PR fulfills these requirements

  • The commit message follows semantic commit message guidelines
  • The changes are documented in component docs and changelog
  • The ESLint plugin has been updated if a new component is added
  • Test have been added or modified, if appropriate
  • Has been verified locally

What kind of change does this PR introduce?

Allows a developer to control the state within useADateRange.

What is the current behavior?

The state setter is not returned from useADateRange:

return {
  value: range,
  onChange,
  minDate,
  maxDate,
};

What is the new behavior (if this is a feature change)?

Exposes the setter from useADateRange to allow a consumer to control it's state directly:

return {
  value: range,
  // Also return the state setter
  setValue: setRange,
  onChange,
  minDate,
  maxDate,
};

Does this PR introduce a breaking change?

No

Other information:

  • Went ahead and refactored the sequencing logic code to improve readability
  • I've rethought the purpose of useADateRange and have created Rethink useADateRange hook #1197 as a result

@brennarvo brennarvo requested a review from guptar8jan April 7, 2022 13:13
@vercel
Copy link

vercel bot commented Apr 7, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/securex/atomic-react/dLNZNxFcYb3QHNfUVJFNAJv6K6vq
✅ Preview: https://atomic-react-git-feat-control-useadaterange-securex.vercel.app

@codecov
Copy link

codecov bot commented Apr 7, 2022

Codecov Report

Merging #1198 (cdd463e) into master (4d8377c) will decrease coverage by 0.70%.
The diff coverage is 100.00%.

❗ Current head cdd463e differs from pull request most recent head 516db8f. Consider uploading reports for the commit 516db8f to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1198      +/-   ##
==========================================
- Coverage   91.20%   90.50%   -0.71%     
==========================================
  Files         104       98       -6     
  Lines        3162     3054     -108     
  Branches     1054     1046       -8     
==========================================
- Hits         2884     2764     -120     
- Misses        240      249       +9     
- Partials       38       41       +3     
Impacted Files Coverage Δ
framework/components/ADatePicker/helpers.js 100.00% <100.00%> (ø)
framework/components/ADatePicker/useADateRange.js 100.00% <100.00%> (ø)
framework/components/ALoader/APageLoader.js 17.64% <0.00%> (-82.36%) ⬇️
framework/components/ALoader/ACiscoLoader.js 33.33% <0.00%> (-66.67%) ⬇️
framework/components/AApp/AApp.js 86.66% <0.00%> (-6.67%) ⬇️
framework/components/AAlert/AAlert.js 82.35% <0.00%> (-2.95%) ⬇️
framework/utils/lorem-ipsum.js
cypress/plugins/index.js
pages/_app.js
pages/_document.js
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0828e63...516db8f. Read the comment docs.

@brennarvo brennarvo changed the title feat: return state setter to control date range hook state feat: enable date range state to be controlled Apr 27, 2022
@vercel
Copy link

vercel bot commented Apr 27, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
atomic-react ✅ Ready (Inspect) Visit Preview Jun 6, 2022 at 7:49PM (UTC)

Copy link
Contributor

@jonathanmcsweet jonathanmcsweet left a comment

Choose a reason for hiding this comment

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

Looks like it's good to go. I couldn't find any major issues and it should be a nice improvement

Comment on lines +13 to +15
return startDate instanceof Date && isSameDate(date, startDate) ||
endDate instanceof Date && isSameDate(date, endDate) ||
false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice simplification here

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.

2 participants