microsoft / xaml-standard

XAML Standard : a set of principles that drive XAML dialect alignment
Other
805 stars 50 forks source link

Use Click instead of Clicked #154

Closed LanceMcCarthy closed 7 years ago

LanceMcCarthy commented 7 years ago

Many Xamarin.Forms event naming conventions have a past tense event name, such as

<Button Clicked="..." />

I propose we standardize the event names to be in the same tense as WPF/UWP/Silverlight style, such as:

<Button Click="..."/>

manuelelucchi commented 7 years ago

agree!

ArchieCoder commented 7 years ago

I would go with OnButtonClick or OnButtonClicked. I know VS uses underscore, but I'm not a fan.

LanceMcCarthy commented 7 years ago

Sebastien, you always keep me on my toes... personally, I don't use underscores on my event handler names either (R# makes this super easy to override).

I'll replace the event handler from my example so that the focus of my example is on the Control's event name.

insinfo commented 7 years ago

agree!

dotMorten commented 7 years ago

@ArchieCoder

I would go with OnButtonClick or OnButtonClicked

That's against the coding guidelines. The "On" prefix is for the protected methods that subclasses can override, and "Button" is already implied from the type, and really the name should match the event which is 'Click' and not 'ButtonClick'.

WRT to the underscores, that's just the VS tooling generating a suggested name for you. This is not part of any standard. If you don't like this, you should submit some Visual Studio feedback to give you an option to change the naming pattern it uses for generating event handler names.

Both WPF and UWP uses 'Click', so makes sense to standardize on that as the original proposal says. Similarly if the XAML standard were to consider the API model as well, it should have a protected virtual void OnClick(...) method.

harinikmsft commented 7 years ago

Button.Click is the plan. See v1draft

birbilis commented 7 years ago

Btw hope Button_Click style for handler naming isn't changed for event handlers (could have option in VS, though not sure if that would work with asp.net auto event handler binding). Those are methods belonging to parent class, eg MyForm.Button2_Click, so "." that some people suggest (in other related thread) to use in naming would confuse reader and potentially compiler too in edge cases. Also would keep "On" for (overridable) virtual methods that fire the event handler, like in Delphi

dotMorten commented 7 years ago

Again underscore has nothing to do with spec or api. You can name them whatever you want. Your beef is with the default suggested name done by your IDE. I never use autocomplete but always name them myself. Button_Click is never sufficient. It would usually be something like OpenFileButton_Click or something, so you're renaming it anyway and can remove the underscore in the process.

birbilis commented 7 years ago

API examples promote a pattern. Better not promote confusing ones. So Button.Click mentioned above had me worried at first, that's why I mentioned it

Btw a clever IDE puts component name in the event handler prefix, not component class, so no need to manually rename anything

dotMorten commented 7 years ago

If you set x:Name VS in fact does do that. But we rarely name all our components. That's s a horrible practice

birbilis commented 7 years ago

Actually I do especially if they're referenced in my code, even when it is implicitly via an event handler. Only when I reuse an event handler at multiple controls I might consider skipping naming them

dotMorten commented 7 years ago

You should only name them if you're referencing them in code. There's an overhead adding names to all. Second if you're are using MVVM as any responsible ☺️ xaml developer, you hardly have any code behind (if any)

Mike-E-angelo commented 7 years ago

As an aside, getting Visual Studio to rename from Button_Click to WhateverClickWhatever seems like a neat VS extension to write. πŸ€“

dotMorten commented 7 years ago

Luckily VS has had that for years ;-) Just F2 or Ctrl+R+R when the cursor is on the method signature (depending on your settings / version)

birbilis commented 7 years ago

Naming just the event handler is implicit naming, but I prefer the explicit one. Event handler is code behind btw. As for names being heavy, it's implementations job to make sure they're not. And no, I don't name containers if not mentioned in the code. But not naming buttons because of some optimizations is bad practice for my taste

birbilis commented 7 years ago

Bit related, if XAML compilation isn't added to the mix, XAML minification step could be added (optional) to the pipeline. Would make it harder to visually debug it, but could map to original xaml lines or at least allow to turn off. That one would strip down unused names, remove newlines and do whatever other optimization

Mike-E-angelo commented 7 years ago

Luckily VS has had that for years ;-)

Hahaha... DAMNYOUYOUKNOWHATIMEANT @dotMorten!!! You would think that this would be a stored setting somewhere. I wonder if ReSharper already has this addressed. πŸ˜›

XAML minification step could be added (optional) to the pipeline

Compilation is definitely preferred. I am not sure you are going to get a great deal of benefit from minification but I like the thinking here, @birbilis. Of course, seeing that word made me think of whitespace, but you're suggesting symbol-renaming as well, correct? It would be a release-mode option much like how Native.NET compilation works.

Is the intent to provide this file as a network resource that is downloaded somewhere? That is what I always dream of and plan for. πŸ˜„ If you are looking to reduce file size, then minification would be valuable. Otherwise I am not so sure of the benefit and would be interested in hearing more.

JerryNixon commented 7 years ago

Well @dotMorten I used to teach that if you are naming your XAML controls you are probably doing it wrong. But I no longer do that. Though {binding} generally makes names unnecessary, there is no cost to a name - none whatsoever. If you do not put an x:Name then the compiler will do it for you. My point is, it is not a horrible practice, but might suggest you are not using XAML to it's potential.

With the introduction of RelativePanel the use of x:Name is more common - it's worth pointing out that I believe that RelativePanel should be the default panel developers use because of its performance. Adaptive triggers and visual states call to controls using their x:Name, too. It makes sense to have x:Name and not to look at it as some kind of XAML problem when it really is not.

Tangential to this topic is x:UID which is used for localization in UWP. This is an amazing subsystem for simplifying localization in apps, and allows for multiple controls to have the same identity for localization purposes while maintaining unique x:Name values for coding purposes. I believe that x:UID should be introduced as the defacto localization strategy for XAML applications. I'll add an issue right now.

dotMorten commented 7 years ago

@jerrynixon note what I said: only do it if you reference them in code. There are plenty valid reasons to do that and it performs better , and MVVM might be "right" but if you think most people do MVVM and not code behind then you don't know your customers. Anyway this is getting off topic.