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

Fix to lookupDispFormUrl #126

Merged
merged 1 commit into from Sep 12, 2018

Conversation

Projects
None yet
3 participants
@prokach
Contributor

prokach commented Sep 12, 2018

Q A
Bug fix? [X ]
New feature? [ ]
New sample? [ ]
Related issues? fixes #X, partially #Y, mentioned in #Z

What's in this Pull Request?

lookupDispFormUrl was fixed in order to provide a correct behavior when user clicks on a lookup field

@estruyf

This comment has been minimized.

Collaborator

estruyf commented Sep 12, 2018

@prokach thanks for the fix.

@estruyf estruyf merged commit 88d2169 into SharePoint:dev Sep 12, 2018

3 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
license/cla All CLA requirements met.

@estruyf estruyf added this to the 1.8.0 milestone Sep 12, 2018

@estruyf estruyf referenced this pull request Sep 12, 2018

Merged

Merge for v1.8.0 #127

@AJIXuMuK

This comment has been minimized.

Contributor

AJIXuMuK commented Sep 18, 2018

I'm not sure about that fix.

Most probably it crashes the functionality.
@prokach could you please describe what exactly wasn't working before that fix?

@prokach

This comment has been minimized.

Contributor

prokach commented Sep 19, 2018

@AJIXuMuK Redirection to a Display Form of an item, which is selected in a lookup field was crashed.

@AJIXuMuK

This comment has been minimized.

Contributor

AJIXuMuK commented Sep 19, 2018

@prokach looks like it didn't work only if a user provides dispUrlForm in props and only if it is provided as .../DispForm.aspx. It other situations it worked correctly.

I've created new PR #133 that addresses both scenarios.

@prokach

This comment has been minimized.

Contributor

prokach commented Sep 19, 2018

@AJIXuMuK Unfortunately, it didn't work even if dispUrlForm is not provided by user in props.
What other situations do you mean? Usually dispUrlForm is provided in props as .../DispForm.aspx, isn't it?

@AJIXuMuK

This comment has been minimized.

Contributor

AJIXuMuK commented Sep 19, 2018

Hi @prokach ,

There are multiple ways to use FieldLookupRenderer and, as a result, work with dispFormUrl value.

  1. Use FieldLookupRenderer control directly and provide dispFormUrl in React props. In that case dispFormUrl can basically contain anything that a user wants.
  2. Use FieldLookupRenderer control directly but instead of providing dispFormUrl pass fieldId. In that case the dispFormUrl will be generated using SPHelper.getLookupFieldListDispFormUrl method. NOTE: this options as well as an option 3 works in Field Customizer but won't work in Web Part.
  3. Use FieldRendererHelper class that automatically "understands" what type of control to use in the specific List Field.

Options 2 and 3 uses the next format for dispFormUrl: ${w.Url}/_layouts/15/listform.aspx?PageType=4&ListId=${f.LookupList}. In that case we need to add additional parameters using ampersands, not question mark as we already have query parameters in the url.

If you look at OOTB implementation of the lookup popup it uses the same format for DisplayForm: listform.aspx?PageType=4.

That's why I've bit updated your fix, that is definitely needed as well, to take into consideration all 3 scenarios of usage and both formats to provide dispFormUrl.

Let me know if it makes sense.

@prokach prokach deleted the prokach:patch-1 branch Sep 20, 2018

@prokach

This comment has been minimized.

Contributor

prokach commented Sep 20, 2018

@AJIXuMuK Thanks a lot for your answer. Now it's really clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment