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

PR: Show the number of matches in the Find/Replace widget #4060

Merged
merged 7 commits into from
Aug 19, 2017

Conversation

mariacamilarg
Copy link
Contributor

@mariacamilarg mariacamilarg commented Jan 27, 2017

It now looks like this:

image

image


Edit:
Fixes #4001.

@mariacamilarg mariacamilarg self-assigned this Jan 27, 2017
@mariacamilarg mariacamilarg added this to the v3.2 milestone Jan 27, 2017
@mariacamilarg
Copy link
Contributor Author

I'm still having trouble to calculate the position of the current search because of how the find is done. If you could give me please some insight I would really appreciate it!

@goanpeca
Copy link
Member

goanpeca commented Feb 1, 2017

@mariacamilaremolinagutierrez what is this PR fixing (add the `Fixes #NNN to the PR description comment on the top)

@ccordoba12 ccordoba12 modified the milestones: v3.2, v3.3 Feb 14, 2017
@ccordoba12 ccordoba12 modified the milestones: v3.2.1, v4.0beta2 Jun 21, 2017
@ccordoba12 ccordoba12 modified the milestones: v3.2.1, v3.2.2 Jul 28, 2017
@ccordoba12 ccordoba12 assigned dalthviz and unassigned mariacamilarg Aug 14, 2017
@ccordoba12
Copy link
Member

@dalthviz, please rebase and finish this one.

@pep8speaks
Copy link

pep8speaks commented Aug 16, 2017

Hello @mariacamilaremolinagutierrez! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on August 19, 2017 at 03:29 Hours UTC

@dalthviz
Copy link
Member

dalthviz commented Aug 16, 2017

@ccordoba12 this PR has an issue already created? I can't find it.
Preview:

With a match:
imagen

Without a match:
imagen

In a WebView:
imagen

@ccordoba12
Copy link
Member

I saw an issue about it but I can't find it right now.

I like it! @goanpeca, what do you say?

@ccordoba12
Copy link
Member

@dalthviz, a test is missing. Please add one.

@goanpeca
Copy link
Member

I like it :-p, can't find the issue either, @dalthviz go ahead and create one?

@ccordoba12
Copy link
Member

@dalthviz, in your screenshots there's too much space between the matches text and the previous/next buttons. Please fix that.

@ccordoba12 ccordoba12 changed the title PR: Ctrl+F gives the number of matches for that search PR: Show the number of matches in the Find/Replace widget Aug 18, 2017
@ccordoba12
Copy link
Member

I like @mariacamilaremolinagutierrez screenshots better :-)

@dalthviz
Copy link
Member

Ok 👍 although I left that space for stuff like this:

imagen

To have a fixed size that doesn't make the search text box resize when the label is long.

@ccordoba12
Copy link
Member

I think it's better to resize the search dialog.

@goanpeca?

@goanpeca
Copy link
Member

I think it's better to resize the search dialog

👍

@ccordoba12
Copy link
Member

@dalthviz, is this ready?

@dalthviz
Copy link
Member

@ccordoba12 yep 👍

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @dalthviz!

@ccordoba12 ccordoba12 merged commit f332818 into spyder-ide:3.x Aug 19, 2017
ccordoba12 added a commit that referenced this pull request Aug 19, 2017
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.

5 participants