Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Making Pages Binding Consistent #6444

Merged
merged 1 commit into from
Jun 27, 2017
Merged

Making Pages Binding Consistent #6444

merged 1 commit into from
Jun 27, 2017

Conversation

rynowak
Copy link
Member

@rynowak rynowak commented Jun 26, 2017

This changeset reckonciles the binding work we did for pages with
controllers.

A quick summary:

@rynowak
Copy link
Member Author

rynowak commented Jun 26, 2017

/cc @pranavkm - this is for you

Copy link
Contributor

@pranavkm pranavkm left a comment

Choose a reason for hiding this comment

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

Is this what the behavior is now:

BindProperty -> Does not bind on GETs by default.
FromRoute (or any other model binding attribute) -> Binds on GETs.

{
get
{
return SupportsGet ? (ActionContext c) => true : (Func<ActionContext, bool>)IsNonGetRequest;
Copy link
Contributor

Choose a reason for hiding this comment

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

Cache the delegates as statics?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

foreach (var requestPredicateProvider in attributes.OfType<IRequestPredicateProvider>())
{
isBindingInfoPresent = true;
bindingInfo.RequestPredicate = requestPredicateProvider.RequestPredicate;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this break after the first item (similar to BindingSource)? It's picking predicate arbitrarily anyway

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm maybe

{
/// <summary>
/// An interface that allows a top-level model to be bound or not bound based on state assocated
/// with the current request.
Copy link
Contributor

Choose a reason for hiding this comment

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

current ActionContext

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I mean the concept of a current request.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also associated

@rynowak
Copy link
Member Author

rynowak commented Jun 26, 2017

BindProperty -> Does not bind on GETs by default.
FromRoute (or any other model binding attribute) -> Binds on GETs.

Yes. Pages and controllers now both behave as you described above.

This changeset reckonciles the binding work we did for pages with
controllers.

A quick summary:
- Moves [BindProperty] to the MVC namespace (#6401)
- Makes [FromRoute] and friends behave consistently (#6402)
- Makes [BindProperty] work with controllers (untracked)
@rynowak rynowak force-pushed the rynowak/binding-enhancements branch from 9036165 to 0ad9c7d Compare June 27, 2017 01:11
@rynowak rynowak merged commit 0ad9c7d into dev Jun 27, 2017
@rynowak rynowak deleted the rynowak/binding-enhancements branch June 27, 2017 01:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants