Skip to content

Conversation

@tjlav5
Copy link
Contributor

@tjlav5 tjlav5 commented Mar 13, 2020

image

Breaking this up into smaller bits... this includes:

  • a basic list view
  • an editor (can only edit values right now)
  • adding field in the preview to maps/arrays
  • deleting fields (preview and editor)

Note on structure, reducer function is shared between Field and Editor, but they each have their own Provided reducer instance. In effect: Field will generally exist within the whole document, while Editor will only have its state initialized to the selected Field's value.

Still to come:

  • better presentation that JSON.stringify in the ListItem (indentation too)
  • changing field type
  • specific field type editors
  • conditionally changing field-key (especially in document-creation/add-field)

@yuchenshi
Copy link
Member

Please kindly rebase this.

Copy link
Member

@yuchenshi yuchenshi left a comment

Choose a reason for hiding this comment

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

LGTM in general, will take another pass and run this locally next Monday.

Copy link
Contributor

@tstirrat tstirrat left a comment

Choose a reason for hiding this comment

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

Looking good

@tjlav5 tjlav5 requested review from tstirrat and yuchenshi March 17, 2020 02:27
@tstirrat
Copy link
Contributor

tstirrat commented Mar 17, 2020

This is awesome btw. So much happening here and such a concise and clear PR. Very easy to grok.

I saw the number of commits and general size and thought "oh lord.. better get a beer".. but it really was super easy to follow.

@tstirrat tstirrat removed their assignment Mar 17, 2020
@tjlav5 tjlav5 merged commit ccdc54b into master Mar 17, 2020
@tjlav5 tjlav5 deleted the firestore-document-fields branch March 17, 2020 16:38
produce((draft, { payload }) => {
const parent = get(draft, getParentPath(payload.path)) || draft;
if (isMap(parent)) {
parent[last(payload.path) as string] = payload.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think last<string>() is more safe, because it will check the type of the input array, vs cast the result.

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.

3 participants