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

Some updates for MaterialTextField & MaterialSelectionControl #411

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

kerberosargos
Copy link

@kerberosargos kerberosargos commented Dec 20, 2020

This pull request includes added ChoiceCommandParameter for MVVM using and changes for TrailingIcon.

Developer can set Command and CommandParamater or Tap event to TrailingIcon for example show or hide password behavior.

Sample update for TrailingIconCommand + Parameter and TrailingIconSelected
Update for TrailingIconSelected
Update for TrailingIcon
Update for ChoiceCommandParameter and TrailingIcon Command + Parameter + Selected Event
Update for counter and choice selected placeholder font size
Update for horizontal orientation width size
Update for counter visibility behavior
@martijn00
Copy link
Member

Can you rebase to include the latest changes? @softlion can you check this PR?

@kerberosargos
Copy link
Author

kerberosargos commented Dec 22, 2020

Can you rebase to include the latest changes? @softlion can you check this PR?

Hello @martijn00 and @softlion I am working new implementations now. I will push 30 minutes later. Please wait to check before this update. Thank you in advance.

@kerberosargos
Copy link
Author

Hello again @martijn00 and @softlion I could push yet. Please check and accept PR. Also thank you for your good plugin.

@martijn00
Copy link
Member

@kerberosargos I think you rebase went wrong, you have conflicts.

@kerberosargos
Copy link
Author

kerberosargos commented Dec 23, 2020

Hello @martijn00 I have merged conflict, please check again.

@kerberosargos kerberosargos changed the title Some updates for MaterialTextField Some updates for MaterialTextField & MaterialSelectionControl Dec 24, 2020
@kerberosargos
Copy link
Author

Hello @martijn00 can you check this PR please?

…te if value is true when text changed event
@martijn00
Copy link
Member

@softlion Can you check this one before i merge it?

@kerberosargos
Copy link
Author

Hello again can you check and merge please?

@kerberosargos
Copy link
Author

Hello @martijn00 and @softlion can you check this pr please?

Copy link
Contributor

@softlion softlion left a comment

Choose a reason for hiding this comment

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

Too much diff changes in the 5 remaining files. Please make sure diffs don't change that much.

@@ -14,7 +14,7 @@ protected override void OnCreate(Bundle savedInstanceState)
ToolbarResource = Resource.Layout.Toolbar;

base.OnCreate(savedInstanceState);
Rg.Plugins.Popup.Popup.Init(this, savedInstanceState);
Rg.Plugins.Popup.Popup.Init(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why has this been removed ?

Copy link
Author

Choose a reason for hiding this comment

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

if you do not remove "savedInstanceState" Rg.Plugins.Popup gives compile error.

Comment on lines +13 to +14
<None Remove="Images\eye-slash-solid-18dp.svg" />
<None Remove="Images\eye-solid-18dp.svg" />
Copy link
Contributor

Choose a reason for hiding this comment

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

remove these lines :)

Copy link
Author

Choose a reason for hiding this comment

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

I have added new icon for password show hide example.


public MaterialTextFieldViewModel()
{
OpenCustomChoiceCommand = new Command(async () =>
{
await MaterialDialog.Instance.AlertAsync("Command tapped. Do what you want ! You can open a custom popup, ask some data, and update the textfield text with this data");
});

CustomChoiceParameterUsingCommand = new Command<MaterialTextField>(async (e) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

a viewmodel should not receive a UI object. Using MaterialTextField, which is a UI object, as a parameter is a very bad practive. Instead create a new class with the interesting parameters only.

To change a property of a ui item, use binding and datatriggers instead.

Copy link
Author

Choose a reason for hiding this comment

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

That is a sample for testing ChoiceCommandParameter. I use this approach for example hide show datepicker for cross platform. Whay do you think that is not good practive?

@@ -17,6 +17,7 @@
<PackageId>XF.Material</PackageId>
<Description>A Xamarin.Forms library for Xamarin.Android and Xamarin.iOS to implement Google's Material Design.</Description>
<DisableFastUpToDateCheck>true</DisableFastUpToDateCheck>
<Version>1.7.7.1</Version>
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this, as it may be set by a batch ?

Copy link
Author

Choose a reason for hiding this comment

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

This is my first pull request. So I am newbie. All comment is for me or @softlion? Thank you for your interest.

Copy link
Contributor

Choose a reason for hiding this comment

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

It’s cool you made a pr

Copy link
Author

Choose a reason for hiding this comment

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

Thank you :)

Copy link
Author

Choose a reason for hiding this comment

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

Hello will you get this pr? @softlion @martijn00

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