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

Razor Components/Blazor access modifiers #11897

Merged
merged 2 commits into from
Apr 9, 2019
Merged

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Apr 8, 2019

Fixes #11010

😅 Lot's 'o lines ... Took longer than I thought!

  • This (mostly) adds the explicit private access modifier.
  • I confirmed samps are happy with the updates.
  • Checked the Razor Syntax topic. It's 🎷 Jamm'in! 🎸.

👮‍♀️ Inconsistency Alert! 👮 We're using attributes (mostly) on the same line in RC/Blazor topics+samps but nowhere else (i.e., model attributes, attribute routing) in docs+samples.

Inconsistent but short ...

@functions {
    [Parameter] public string MyString { get; set; }
    [Parameter] public string AnotherString { get; set; }

    ...
}

Consistent with docs+samples elsewhere but long ...

@functions {
    [Parameter]
    public string MyString { get; set; }

    [Parameter]
    public string AnotherString { get; set; }

    ...
}

How do u want to play it? cc: @NTaylorMullen - Thoughts?

@guardrex guardrex requested a review from danroth27 April 8, 2019 18:23
@NTaylorMullen
Copy link
Contributor

I'll let @danroth27 comment on this one 😄

@danroth27
Copy link
Member

danroth27 commented Apr 8, 2019

@guardrex We should be put the [Parameter] attribute above the property.

@Rick-Anderson
Copy link
Contributor

@danroth27

@guardrex We should be the [Parameter] attribute above the property.

Should that be

@guardrex We should have the [Parameter] attribute above the property.

@guardrex
Copy link
Collaborator Author

guardrex commented Apr 9, 2019

... and while ur answering ... add it here? I was about to move on to the EventCallback issue, and I think we should get this reviewed and merged before I dig into that one. I was planning to open a new issue for "[Parameter] on its own line" and work it separately if that's what we're going to do.

@guardrex
Copy link
Collaborator Author

guardrex commented Apr 9, 2019

@Rick-Anderson Let's go ahead. The samps run and the changes are fairly straightforward here. I need this one merged in order to get on to the EventCallback issue (aspnet/AspNetCore.Docs #11243).

I opened Razor Components/Blazor [Parameter] attribute (aspnet/AspNetCore.Docs #11913) to deal with [Parameter] on its own line. Will dig into that later.

@guardrex guardrex requested review from Rick-Anderson and removed request for danroth27 April 9, 2019 21:16
@guardrex guardrex merged commit f31361b into master Apr 9, 2019
@delete-merged-branch delete-merged-branch bot deleted the guardrex/access-mods branch April 9, 2019 21:19
@danroth27
Copy link
Member

Sorry about my typo 😝.

I was trying to say that instead of this:

[Parameter] public string MyString { get; set; }

We should at some point switch to this:

[Parameter]
public string MyString { get; set; }

@guardrex
Copy link
Collaborator Author

guardrex commented Apr 9, 2019

@danroth27 Gotcha ... opened #11913 to address it.

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.

4 participants