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

Fix to lookupDispFormUrl #126

Merged
merged 1 commit into from
Sep 12, 2018
Merged

Fix to lookupDispFormUrl #126

merged 1 commit into from
Sep 12, 2018

Conversation

prokach
Copy link
Contributor

@prokach 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
Copy link
Member

estruyf commented Sep 12, 2018

@prokach thanks for the fix.

@estruyf estruyf merged commit 88d2169 into pnp:dev Sep 12, 2018
@estruyf estruyf added this to the 1.8.0 milestone Sep 12, 2018
@estruyf estruyf mentioned this pull request Sep 12, 2018
@AJIXuMuK
Copy link
Collaborator

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
Copy link
Contributor Author

prokach commented Sep 19, 2018

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

@AJIXuMuK
Copy link
Collaborator

@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
Copy link
Contributor Author

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
Copy link
Collaborator

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 patch-1 branch September 20, 2018 06:26
@prokach
Copy link
Contributor Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants