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: add dateTime knob #940

Merged

Conversation

logickoder
Copy link
Contributor

Adds a DateTime Knob

List of issues which are fixed by the PR

#899

Screenshots

knobs_example.2023-10-06.15-15-33.mp4

Checklist

  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making].
  • All existing and new tests are passing.

If you need help, consider asking for advice on Discord.

…logickoder/feat/date-time-knob

# Conflicts:
#	examples/knobs_example/lib/home_page.dart
#	examples/knobs_example/lib/widgetbook.dart
#	packages/widgetbook/lib/src/fields/field_type.dart
#	packages/widgetbook/lib/src/knobs/builders/knobs_builder.dart
@logickoder
Copy link
Contributor Author

Hi @YoussefRaafatNasry can you please take a look/review?

@YoussefRaafatNasry YoussefRaafatNasry changed the title feat(knobs): added date_time_knob feat: add dateTime knob Oct 8, 2023
@YoussefRaafatNasry YoussefRaafatNasry linked an issue Oct 8, 2023 that may be closed by this pull request
Copy link
Member

@YoussefRaafatNasry YoussefRaafatNasry left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your time and effort on this new knob 💙
Many people will find this useful! You have done a good job covering everything from docs to tests.

Because DateTime is one of the complicated data types, I've added some comments to simplify and improve the implementation a bit.

I will review this again once we get these comments done, because the implementation will change a bit. Let me know if you need any further help or any more info on any of the comments.

docs/knobs/example.mdx Outdated Show resolved Hide resolved
examples/knobs_example/lib/home_page.dart Outdated Show resolved Hide resolved
packages/widgetbook/lib/src/fields/date_time_field.dart Outdated Show resolved Hide resolved
packages/widgetbook/lib/src/fields/date_time_field.dart Outdated Show resolved Hide resolved
packages/widgetbook/lib/src/fields/date_time_field.dart Outdated Show resolved Hide resolved
packages/widgetbook/lib/src/fields/date_time_field.dart Outdated Show resolved Hide resolved
packages/widgetbook/lib/src/fields/date_time_field.dart Outdated Show resolved Hide resolved
@logickoder
Copy link
Contributor Author

Hi @YoussefRaafatNasry I have made some changes, can you review again?

Copy link
Member

@YoussefRaafatNasry YoussefRaafatNasry left a comment

Choose a reason for hiding this comment

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

Good job with fixing all the review comments!
I have some minor changes that are a follow-up for the previous review.

packages/widgetbook/lib/src/fields/date_time_field.dart Outdated Show resolved Hide resolved
packages/widgetbook/lib/src/fields/date_time_field.dart Outdated Show resolved Hide resolved
packages/widgetbook/lib/src/fields/date_time_field.dart Outdated Show resolved Hide resolved
packages/widgetbook/lib/src/fields/date_time_field.dart Outdated Show resolved Hide resolved
…logickoder/feat/date-time-knob

# Conflicts:
#	packages/widgetbook/CHANGELOG.md
@logickoder
Copy link
Contributor Author

Hi @YoussefRaafatNasry I have made some changes, can you review again?

Copy link
Member

@YoussefRaafatNasry YoussefRaafatNasry left a comment

Choose a reason for hiding this comment

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

Great job @logickoder! 💙

@YoussefRaafatNasry YoussefRaafatNasry merged commit 48481e8 into widgetbook:main Oct 10, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DateTime Knob
2 participants