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

Allow default metadata fields to be overridden and edited in place #597

Merged
merged 8 commits into from
May 18, 2020
Merged

Allow default metadata fields to be overridden and edited in place #597

merged 8 commits into from
May 18, 2020

Conversation

i-a-n
Copy link
Contributor

@i-a-n i-a-n commented May 5, 2020

Resolves #596

override-revert

First PR, happy to receive any additional pushes, and sorry for any inconsistencies.

This allows a user to see the default fields for a new or existing document, choose to "override" the defaults, and revert back to the default at any time. The way I accomplished this in general was by removing the StaticMetaFields component, and instead allowing MetaFields to display both default and additional fields. By checking the redux store (specifically the metadata: { metadata:{} } object), the MetaFields component will render each field as either a Static or regular MetaField, and by passing in props, each subcomponent is able to tell whether it is a default field, and whether it is currently in "default mode".

Choosing to "override" a default simply adds that key/value to the metadata redux object, allowing it to be rendered as a regular MetaField. "Reverting" it simply removes it, and it goes back to being rendered as a StaticMetaField.

Note: I updated snapshot testing and added action/reducer tests, but added no unit tests. I'd appreciate any help or direction on that, if you feel we need them. Thanks again.

@ashmaroli
Copy link
Member

Good job, @i-a-n! The code looks good to me on first glance..

Copy link
Member

@mertkahyaoglu mertkahyaoglu left a comment

Choose a reason for hiding this comment

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

Thanks for the pr @i-a-n ❤️ This is really good.

I have 2 comments. After those are addressed, we can merge this 🎉

src/components/metadata/StaticMetaButton.js Outdated Show resolved Hide resolved
src/components/metadata/MetaButtons.js Outdated Show resolved Hide resolved
@i-a-n i-a-n requested a review from mertkahyaoglu May 16, 2020 22:37
@mertkahyaoglu
Copy link
Member

@i-a-n We can merge this after tests are passing.

@ashmaroli
Copy link
Member

Thank you @i-a-n

@ashmaroli ashmaroli merged commit 3042a3d into jekyll:master May 18, 2020
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.

Is default, editable metadata on a new document possible?
3 participants