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

Feature/ComboBox #414

Merged
merged 29 commits into from
May 23, 2020
Merged

Feature/ComboBox #414

merged 29 commits into from
May 23, 2020

Conversation

fergusonr
Copy link
Contributor

Simple text field with auto-complete. List Demos -> Search Single Item

https://drive.google.com/open?id=1uxh5a6y8kqIv6ZMdy6-k_CJQWv1bhz_y

@BDisp
Copy link
Collaborator

BDisp commented May 15, 2020

Good job @fergusonr. My advice is to change this code:

this.Enter += (object sender, EventArgs e) => {
	this.SetFocus (search);
	search.CursorPosition = search.Text.Length;
};

to this one:

public override bool OnEnter ()
{
	if (!search.HasFocus)
		this.SetFocus (search);
	search.CursorPosition = search.Text.Length;

	return true;
}

and let the Enter event to the end user. That was the reason why I changed that.

@fergusonr
Copy link
Contributor Author

Thanks @BDisp change checked-in

@fergusonr
Copy link
Contributor Author

fergusonr commented May 15, 2020

Hi @BDisp (I think) your last check-in has changed some behaviour of TextField.Text

If I programatically set the Text value, it only takes effect on the second assignment

search.Text = "lost"; // does not set
search.Text = "found"; // this one works

@BDisp
Copy link
Collaborator

BDisp commented May 15, 2020

@fergusonr you have both values of the TextField.Text in the event. The new one through ((TextField)sender).Text and the old one through the e. So I think you must deal with this two.

@BDisp
Copy link
Collaborator

BDisp commented May 15, 2020

@fergusonr that is breaking in the linux. Since this project is multi-platform you must check that.

@BDisp
Copy link
Collaborator

BDisp commented May 17, 2020

@fergusonr it's still breaking in the linux. See the DirListView implementeation on the File->Open. You are dealing folders with a Windows's way only that not is compatible with Unix. Use WSL and install dotnet, mono and vsdbg on it to run your project on linux.

@BDisp
Copy link
Collaborator

BDisp commented May 18, 2020

Well done. It's working in both now. I haven't macOS but as it is based Unix OS, it must working there too.

@fergusonr
Copy link
Contributor Author

@BDisp these should be my last check-ins. Tidy up behaviour when hosted in a Window

https://drive.google.com/open?id=1nS6GY8qSW9Ka4TcmtMvQ46whdTOnRB-X

@BDisp
Copy link
Collaborator

BDisp commented May 20, 2020

That's great. Your control is near to be a combo that you can add properties to a read only feature too. How about that? :-)

@fergusonr fergusonr changed the title Feature/TextFieldAutoComplete Feature/ComboBox May 21, 2020
@BDisp
Copy link
Collaborator

BDisp commented May 21, 2020

@fergusonr don't forget to rebase your branch to reflect the changes in the master for the files you are modifying.

@tig
Copy link
Collaborator

tig commented May 22, 2020

@fergusonr Can you please implement a Scenario on UI Catalog that exercises this control as part of this PR?

See instructions here: https://github.com/migueldeicaza/gui.cs/tree/master/UICatalog

@fergusonr
Copy link
Contributor Author

fergusonr commented May 22, 2020

@tig that's done. My scenario appears at the end for some reason I cannot immediately figure out

Also looks like your change "UI catalog (#387)" to Label.cs has broken Label positioning. See my Lists scenario.

@tig tig mentioned this pull request May 22, 2020
@tig
Copy link
Collaborator

tig commented May 22, 2020

This is very cool.

I think that change to Label was done by @BDisp and I just merged it as part of that PR.

image

See: #497

If you start typing in the edit and then pick an item, I would think the list would update to reflect that the edit now has that item in it full.

image

E.g. in the above the list should JUST have debian_version in it.

Also, when open it's not clear that it's an edit and a list. I wonder if making the edit one or two char narrower would help?

image

Or... use a different color for the list? Or both:

image

@fergusonr
Copy link
Contributor Author

fergusonr commented May 22, 2020

Thanks @tig agree the list should be differentiated from search. I do not change the list on selection because that's the behaviour of the Windows ComboBox. ie: it disappears on selection (unless I'm misunderstand you)

All said, I do not have a strong opinion either way, so if this gets merged to master, its open to public PR's

@tig
Copy link
Collaborator

tig commented May 23, 2020

What about my suggestion for making the list narrower than the edit?

@BDisp
Copy link
Collaborator

BDisp commented May 23, 2020

A suggestion may even be of variable size within a minimum and maximum limit depending on the size of the item.

@tig
Copy link
Collaborator

tig commented May 23, 2020

Also, please add

///<inheritdoc cref="name"/>

comments to all of your public overrides. This will ensure we don't have any warnings on build.

e.g.

///<inheritdoc cref="OnEnter"/>
public override bool OnEnter ()
		{
			if (!search.HasFocus)
				this.SetFocus (search);

			search.CursorPosition = search.Text.Length;

			return true;
		}

@fergusonr
Copy link
Contributor Author

@tig ok, thats done.

@BDisp
Copy link
Collaborator

BDisp commented May 23, 2020

@fergusonr this is getting fantastic. I'm already visualizing the ComboBox with a 'V' on the right and it is possible to open and close the search list :-)

@tig
Copy link
Collaborator

tig commented May 23, 2020

I verified this is AWESOME by fetching the PR locally.

Great freaking job!

@tig tig merged commit e3e7c29 into gui-cs:master May 23, 2020
@BDisp
Copy link
Collaborator

BDisp commented May 23, 2020

Really he is doing a fabulous job. I'm really amazed.

@tig tig mentioned this pull request Jun 4, 2020
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.

None yet

3 participants