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

feat: Introducing Header and Placeholder #1014

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

eriklimakc
Copy link
Contributor

GitHub Issue: #974 #842

PR Type

What kind of change does this PR introduce?

  • Feature

Description

Introducing Header as Label and Placeholder to TextBox.

The former PlaceholderText was replaced by Header and a new Placeholder was added. The goal is to reproduce the Material 3 Textbox behavior.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Commits must be following the Conventional Commits specification
  • Tested UWP
  • Tested iOS
  • Tested Android
  • Tested WASM
  • Tested GTK
  • Tested MacOS
  • Contains No breaking changes

    If the pull request contains breaking changes, commit message must contain a detailed description of the action to take for the consumer of this library. As explained by the Conventional Commits specification

@eriklimakc eriklimakc self-assigned this May 30, 2023
@github-actions
Copy link

github-actions bot commented May 30, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

1 similar comment
@github-actions
Copy link

github-actions bot commented May 30, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

@eriklimakc
Copy link
Contributor Author

Windows

image
image
image
image

WASM

image
image
image
image

GTK

Gtk cursor seems to be a bit bugged
image
image
image
image

Android

image
image
image
image

Android Obs

It feels like Android TextBox Header and Placeholder are a bit below of the center when unfocused. Do you feel the same? Would we add a proper Margin/Padding/Alignment only for Android to fix it?

image

@kazo0 @agneszitte @Xiaoy312

@agneszitte
Copy link
Contributor

While testing the azure static web link, seems that the icon is not properly aligned vertically
image

@eriklimakc
Copy link
Contributor Author

While testing the azure static web link, seems that the icon is not properly aligned vertically

image

Thanks @agneszitte, will fix it ☺️

@github-actions
Copy link

github-actions bot commented Jun 1, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

@eriklimakc
Copy link
Contributor Author

I'm having a problem with the TextBox cursor:

When I have Placeholder and Header the ContentElement_CompositeTransform.TranslateY (cursor position) has value of 8 which means the cursor is aligned with the Placeholder position like this:

image

But when I have only Header the cursor should start with the position 0, which means it would be aligned with the Header text and when I type something it would go down (position 8). But ContentElement_CompositeTransform.TranslateY (cursor position) is starting with value 8 already, like this:

image

When I type:

image

I can't find a way of check Text, PlaceholderText and Header at the same time to manipulate the cursor position as desired. Do you guys have any ideas?

I have pushed my changes.

@agneszitte @kazo0

@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

@agneszitte
Copy link
Contributor

@eriklimakc I will pull your branch to take a look and help you fix this issue

@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

@github-actions
Copy link

github-actions bot commented Jun 6, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

@agneszitte
Copy link
Contributor

agneszitte commented Jun 6, 2023

@eriklimakc I have pushed on your branch a commit to fix the MaterialFilledTextBoxStyle so it can follow the proper specs we agreed on for the related issue.

Here some things to note:

  • The style follows the specs and works appropriately on Windows at the moment if I did not miss anything
    (I quickly added more samples so it is easy to test all the scenarios for MaterialFilledTextBoxStyle)
  • I quickly tested the style on the other platforms and on specific cases we have some issues, so you will need to check them more closely. Since it works properly on Windows, the goal will be to determine what is not working on the Uno side.
  • You will be able to use the same logic for the MaterialOutlinedTextBoxStyle
  • Naming for the converter I added (+ related resources) will need to be adjusted for something better at the end if it will be the way to go for the final changes

(cc @kazo0)

@agneszitte
Copy link
Contributor

agneszitte commented Jun 6, 2023

@eriklimakc I have pushed on your branch a commit to fix the MaterialFilledTextBoxStyle so it can follow the proper specs we agreed on for the related issue.

Here some things to note:

  • The style follows the specs and works appropriately on Windows at the moment if I did not miss anything
    (I quickly added more samples so it is easy to test all the scenarios for MaterialFilledTextBoxStyle)
  • I quickly tested the style on the other platforms and on specific cases we have some issues, so you will need to check them more closely. Since it works properly on Windows, the goal will be to determine what is not working on the Uno side.
  • You will be able to use the same logic for the MaterialOutlinedTextBoxStyle
  • Naming for the converter I added (+ related resources) will need to be adjusted for something better at the end if it will be the way to go for the final changes

(cc @kazo0)

As an additional note, the converter is working fine on Windows but for the other platforms, the converter is only trigged once for each state. It seems like once it is triggered it won't trigger anymore for that state.

@agneszitte
Copy link
Contributor

Related Issue for the Converter/VisualStates issue:
unoplatform/uno#12556

@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

2 similar comments
@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

@github-actions
Copy link

github-actions bot commented Sep 12, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

@eriklimakc
Copy link
Contributor Author

eriklimakc commented Sep 12, 2023

Regarding animations all seems to be working fine after unoplatform/uno#12556.

Only getting this behavior on non-windows platforms:

When TextBox is pressed and Placeholder/Text + Header is present they get back to the center and then animates to their right position:

TextBox.PnH.issue.mp4

@kazo0
Copy link
Collaborator

kazo0 commented Sep 27, 2023

@eriklimakc what's the current status of this? Any remaining blockers?

@eriklimakc
Copy link
Contributor Author

Regarding animations all seems to be working fine after #974.

Only getting this behavior on non-windows platforms:

When TextBox is pressed and Placeholder/Text + Header is present they get back to the center and then animates to their right position:

TextBox.PnH.issue.mp4

@kazo0, there is only this behavior on non-windows platforms that I mentioned above with the video.

@eriklimakc eriklimakc force-pushed the dev/ERLI/974-LabelAndPlaceholderTextbox branch from 7daf3cd to 7c22d38 Compare November 2, 2023 17:36
Copy link

github-actions bot commented Nov 2, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

@eriklimakc
Copy link
Contributor Author

Skia X Win

Skia_vs_Win_issue

Apparently when the animation code is executed, the Text, Header, and Placeholder are quickly set to their default state and after that they are animated.

In order to test I replaced the animation code for Setters like below and we don't have that behavior.

<Setter Target="HeaderElement_CompositeTransform.TranslateY"
	Value="-11" />
<Setter Target="ContentElement_CompositeTransform.TranslateY"
	Value="{Binding RelativeSource={RelativeSource TemplatedParent}, Converter={StaticResource EmptyToFocusedContentCompositeTransformTranslateY}, TargetNullValue=0, FallbackValue=0}" />
<Setter Target="PlaceholderElement_CompositeTransform.TranslateY"
	Value="{Binding RelativeSource={RelativeSource TemplatedParent}, Converter={StaticResource EmptyToFocusedContentCompositeTransformTranslateY}, TargetNullValue=0, FallbackValue=0}" />
<Setter Target="HeaderElement_CompositeTransform.ScaleX"
	Value="0.7" />
<Setter Target="HeaderElement_CompositeTransform.ScaleY"
	Value="0.7" />

cc @Xiaoy312 @kazo0 @agneszitte

@Xiaoy312
Copy link
Contributor

Xiaoy312 commented Nov 3, 2023

@eriklimakc re: #1014 (comment)
At a glance, this seems like an issue with the animation not picking up the current set/animated value as the starting point for the (start...end) animation. This is not probably not something we will fix here, but in uno.
Can you can open an issue in uno and @ me there.
Also, is this happening on other platforms as well, like ios/droid or wasm?

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

1 similar comment
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

@eriklimakc eriklimakc requested a review from kazo0 May 23, 2024 11:29
@eriklimakc eriklimakc force-pushed the dev/ERLI/974-LabelAndPlaceholderTextbox branch from 96779c1 to 6b8beaa Compare May 23, 2024 15:32
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

1 similar comment
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

src/library/Uno.Material/Styles/Controls/v2/TextBox.xaml Outdated Show resolved Hide resolved
Comment on lines +26 to +27
<um:FromTextBoxEmptyStringToValueConverter x:Key="PlaceholderContentCompositeTransformTranslateY"
HeaderAndPlaceholderValue="8"
Copy link
Contributor

Choose a reason for hiding this comment

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

any chance that you are using tab with a size of 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not me, but the solution, apparently:

indent_size = 2

I tried to follow the pattern:

image

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1014.eastus2.azurestaticapps.net

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.

[Material][TextBox][PasswordBox] Add support for Label and Placeholder at the same time
4 participants