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 ListContext to ReferenceArrayInput #5886

Merged
merged 16 commits into from Feb 24, 2021

Conversation

djhi
Copy link
Contributor

@djhi djhi commented Feb 8, 2021

  • Implementation
  • Ability to select rows to change the input value
  • Tests
  • Documentation

@djhi djhi added the WIP Work In Progress label Feb 8, 2021
@djhi djhi force-pushed the reference-array-input-list-context branch from 44406e2 to 7eeb242 Compare February 9, 2021 15:28
@djhi djhi added RFR Ready For Review and removed WIP Work In Progress labels Feb 12, 2021
@fzaninotto
Copy link
Member

This should be PRed against next

@djhi djhi force-pushed the reference-array-input-list-context branch from d5aace6 to 8382e14 Compare February 22, 2021 09:41
@djhi djhi changed the base branch from master to next February 22, 2021 09:41
const context = useContext(ReferenceArrayInputContext);

if (props.choices) {
return props;
Copy link
Member

Choose a reason for hiding this comment

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

Please use a more fine-grained approach, as in useListContext in master

@@ -0,0 +1,9 @@
import { Record, RecordMap } from '../types';

export const indexById = (records: Record[] = []): RecordMap =>
Copy link
Member

Choose a reason for hiding this comment

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

please add jsDoc with sample input/output

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

const listContext = useMemo(
() => ({
...controllerProps,
setSort: controllerProps.setSortForList,
Copy link
Member

Choose a reason for hiding this comment

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

Please add a jsDoc explaining why you do some voodoo here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

/>
<ResourceContextProvider value={props.reference}>
<ReferenceArrayInputContextProvider value={controllerProps}>
<ListContextProvider value={listContext}>
Copy link
Member

Choose a reason for hiding this comment

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

I don't get it. What's the use of a ReferenceArrayInputContext if all you need is already in the ListContext?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree it is mostly redundant now but it does provide the warning prop which indicates non "fatal" errors regarding references.

@fzaninotto fzaninotto merged commit c4be146 into next Feb 24, 2021
@fzaninotto fzaninotto deleted the reference-array-input-list-context branch February 24, 2021 09:06
@fzaninotto fzaninotto added this to the 3.13 milestone Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants