Skip to content
This repository has been archived by the owner on Mar 8, 2024. It is now read-only.

Use Click instead of Clicked #154

Closed
LanceMcCarthy opened this issue May 24, 2017 · 19 comments
Closed

Use Click instead of Clicked #154

LanceMcCarthy opened this issue May 24, 2017 · 19 comments

Comments

@LanceMcCarthy
Copy link

LanceMcCarthy commented May 24, 2017

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

agree!

@ArchieCoder
Copy link

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

@LanceMcCarthy
Copy link
Author

LanceMcCarthy commented May 24, 2017

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

insinfo commented May 24, 2017

agree!

@dotMorten
Copy link

dotMorten commented May 24, 2017

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

Button.Click is the plan. See v1draft

@birbilis
Copy link

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

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

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

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

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

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

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

@dotMorten
Copy link

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

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

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

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

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

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants