Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

Conversation

@adrianknight89
Copy link
Contributor

@adrianknight89 adrianknight89 commented May 12, 2017

Description of Change

I have a debug setup where setting soft input mode at startup takes 14 ms for AdjustPan and 52 ms for AdjustResize. For most apps, the landing page should not contain any input views, hence, the input mode should not be needed to be set immediately.

A platform-specific property was added to skip calling SetSoftInputMode(). According to Android documentation, the default setting for soft input mode is unspecified.

I haven't made documentation changes (yet) since I'm not sure if this PR is okay.

Please make sure to document this here. :)

Bugs Fixed

  • N/A

API Changes

Added:

  • platform-specific ShouldSetWindowSoftInputModeAtStartup
  • Unspecified enum value for soft input mode

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense


public enum WindowSoftInputModeAdjust
{
Unspecified,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this should be at the top or bottom. Currently Pan is the default value.

public static readonly BindableProperty WindowSoftInputModeAdjustProperty =
BindableProperty.Create("WindowSoftInputModeAdjust", typeof(WindowSoftInputModeAdjust),
typeof(Application), WindowSoftInputModeAdjust.Pan);
public static readonly BindableProperty WindowSoftInputModeAdjustProperty = BindableProperty.Create(nameof(WindowSoftInputModeAdjust), typeof(WindowSoftInputModeAdjust), typeof(Application), WindowSoftInputModeAdjust.Pan);
Copy link
Contributor Author

@adrianknight89 adrianknight89 May 12, 2017

Choose a reason for hiding this comment

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

Since Unspecified was added, it might make sense to rename this property to WindowSoftInputMode (get rid of Adjust at the end). I think a breaking change here should be tolerated, but I haven't made any changes.

if (Xamarin.Forms.Application.Current.OnThisPlatform().GetShouldSetWindowSoftInputModeAtStartup())
SetSoftInputMode();
else
Xamarin.Forms.Application.Current.OnThisPlatform().UseWindowSoftInputModeAdjust(WindowSoftInputModeAdjust.Unspecified);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default to Unspecified since XF is currently defaulting to Pan and we do not want that.

@hartez hartez self-assigned this May 12, 2017
@adrianknight89
Copy link
Contributor Author

I'm thinking if you could reduce startup times significantly with the current XF roadmap, perhaps this change isn't needed otherwise we should look into making soft input mode optional as in this PR.

Copy link
Contributor

@samhouts samhouts left a comment

Choose a reason for hiding this comment

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

I like that you've taken great care to avoid making a breaking behavior change. I'm wondering if we can achieve the same result by just adding the Unspecified option to the end of the enum (to avoid a breaking ABI change) and updating our documentation to suggest that users set the value of WindowSoftInputModeAdjust to Unspecified to take advantage of the performance gain. This would eliminate the need for the new PS. Thoughts?

@adrianknight89
Copy link
Contributor Author

updating our documentation to suggest that users set the value of WindowSoftInputModeAdjust to Unspecified

By the time they set this manually, we will have called SetSoftInputMode();, right? The point of ShouldSetWindowSoftInputModeAtStartupProperty was to toggle this behavior on and off when the application is loading. If the flag is false, then Unspecified is used. By default, it's true and we use Pan.

@adrianknight89
Copy link
Contributor Author

Android documentation on adjustUnspecified

It is unspecified whether the activity's main window resizes to make room for the soft keyboard, or whether the contents of the window pan to make the current focus visible on-screen. The system will automatically select one of these modes depending on whether the content of the window has any layout views that can scroll their contents. If there is such a view, the window will be resized, on the assumption that scrolling can make all of the window's contents visible within a smaller area.
This is the default setting for the behavior of the main window.

In my opinion, XF should have refrained from setting this for users since this is a configurable option depending on user needs. I'm not sure why either Pan or Resize needs to be set right off the bat. If we set this to Unspecified by default (by removing the call to SetSoftInputMode()), I'm not sure if this would be a big/medium/small breaking change.

@samhouts
Copy link
Contributor

samhouts commented Nov 8, 2017

By the time they set this manually, we will have called SetSoftInputMode();,

So, when will they have set ShouldSetWindowSoftInputModeAtStartupProperty?

@adrianknight89
Copy link
Contributor Author

Since this is an application-level platform specifics, it would have to be called in App.cs before LoadApplication() is invoked.

@samhouts
Copy link
Contributor

samhouts commented Nov 9, 2017

WindowSoftInputModeAdjustProperty is also an application-level platform specific. If it is set to Unspecified by the user in App.cs before LoadApplication is invoked, would it not have the same effect? Your changes to the Activity are also necessary for this to work, of course, but I still think we can get away without the additional PS. I could be missing something, though, so please forgive me if I'm being dense. Just trying to understand.

@adrianknight89
Copy link
Contributor Author

If we set it to Unspecified, we're still executing Window.SetSoftInputMode(adjust); inside SetSoftInputMode() and I'm not sure if this costs us time. Since I'm running VS2017 now and XF Android seems to have compilation issues there, I cannot check this. My changes were skipping the method call altogether at startup. Alternatively, Unspecified can skip SetSoftInputMode() the first time it's called:

// inside LoadApplication()

if(Xamarin.Forms.Application.Current.OnThisPlatform().GetWindowSoftInputModeAdjust() != WindowSoftInputModeAdjust.Unspecified)
    SetSoftInputMode();

but for subsequent calls (i.e. AppOnPropertyChanged), the logic should account for the new value:

default:
    adjust = SoftInput.AdjustUnspecified;
    break; 

Let me know if this works.

@samhouts
Copy link
Contributor

samhouts commented Dec 6, 2017

@adrianknight89: You may have noticed that we're now VS 2017 compatible! Yay! I hope you're able to compile now. If not, please let us know.

Yes, I think we're on the same page about skipping that call if the initial value is Unspecified. That seems to make sense to me.

Thank you!

@adrianknight89
Copy link
Contributor Author

@samhouts Thanks. I did a fresh install of Win10 and VS2017 (15.5) (which I'd been planning on). Right now, I'm having issues cloning the XF project through the VS Git extension.

Remote: Compressing objects: 100% (43/43), done.        
Remote: Total 24193 (delta 22), reused 3 (delta 3), pack-reused 24147        
Remote: Repository not found.
Error encountered while cloning the remote repository: Git failed with a fatal error.
repository 'https://github.com/xamarin/Xamarin.Forms.Build/' not found
clone of 'https://github.com/xamarin/Xamarin.Forms.Build' into submodule path 'C:/Users/xyz/source/repos/Xamarin.Forms/Xamarin.Forms.Build' failed
Failed to clone 'Xamarin.Forms.Build'. Retry scheduled
Cloning into 'C:/Users/xyz/source/repos/Xamarin.Forms/Xamarin.Forms.Build'...
remote: Repository not found.
repository 'https://github.com/xamarin/Xamarin.Forms.Build/' not found
clone of 'https://github.com/xamarin/Xamarin.Forms.Build' into submodule path 'C:/Users/xyz/source/repos/Xamarin.Forms/Xamarin.Forms.Build' failed
Failed to clone 'Xamarin.Forms.Build' a second time, aborting

@rmarinho
Copy link
Member

rmarinho commented Dec 6, 2017

@adrianknight89 can you try the command line ? should allow to clone and skipping submodules.

@adrianknight89
Copy link
Contributor Author

@rmarinho Thanks. Excluding the submodules worked. However, I'm still unable to compile the UI test project for Android.

Severity	Code	Description	Project	File	Line	Suppression State
Error	MSB4062	The "Xamarin.Forms.Build.Tasks.XamlGTask" task could not be loaded from the assembly C:\Users\xyz\source\repos\Xamarin.Forms\.nuspec\Xamarin.Forms.Build.Tasks.dll. Could not load file or assembly 'file:///C:\Users\xyz\source\repos\Xamarin.Forms\.nuspec\Xamarin.Forms.Build.Tasks.dll' or one of its dependencies. The system cannot find the file specified. Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask.	Embedding.XF	C:\Users\xyz\source\repos\Xamarin.Forms\.nuspec\Xamarin.Forms.targets	56	

@adrianknight89
Copy link
Contributor Author

adrianknight89 commented Dec 7, 2017

This is being thrown in Embedding.XF, PagesGallery, and Xamarin.Forms.Controls. File: Xamarin.Forms.targets

@samhouts
Copy link
Contributor

samhouts commented Dec 7, 2017

You might try deleting any .dll files in the .nuspec folder (run a Clean Solution and close VS first so it won't complain), then restart VS, and rebuild. You may need to rebuild the Xaml.Build.Tasks first. This little dance sometimes helps.

@adrianknight89
Copy link
Contributor Author

@samhouts Thanks. Your suggestions worked! I think I needed to rebuild Xaml.Build.Tasks.

mattleibow pushed a commit that referenced this pull request Jan 28, 2021
* Fixes #917, make sure to dispose after javacast

* C# 8 madness!

* ShareFile: describe contentType ctor arg

This is related to https://github.com/MicrosoftDocs/xamarin-docs/issues/2246

* Clarify ContentType API (#958)

* Clarify ContentType API

* Clarify content type API

* Clarify Content Type api
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants