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

Add support for Zotero's Date property #77

Merged
merged 2 commits into from
May 22, 2022

Conversation

samirunni
Copy link
Contributor

@samirunni samirunni commented May 14, 2022

  • Followed same approach as Add abstract field #35
  • Tested locally and confirmed to be working
  • Went with a text type for the date instead of a date type because Zotero itself doesn't store that property as a date
    • For example, my test entries (journal articles) included dates of 2022-05
    • It makes the most sense for users to use a formula column on the Notion side to parse the date strings

Copy link
Owner

@dvanoni dvanoni left a comment

Choose a reason for hiding this comment

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

@samirunni thanks so much for your contribution! 🙇

I completely agree with your decision to use a text property instead of a date property. I even have some items that only have the year in the date field, so text works perfectly.

I requested one minor change, but I tested locally and it seems to work great!

content/notero-item.ts Outdated Show resolved Hide resolved
@dvanoni
Copy link
Owner

dvanoni commented May 22, 2022

I went ahead and applied that small change. Thanks again for the contribution!

@dvanoni dvanoni merged commit ce404dd into dvanoni:main May 22, 2022
@samirunni samirunni deleted the su/add-date-support branch May 23, 2022 01:10
@samirunni
Copy link
Contributor Author

@dvanoni - sorry, didn't see your prior comment until now. Thanks for merging!

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.

None yet

2 participants