-
Notifications
You must be signed in to change notification settings - Fork 14
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
improve search in document #160
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
THere's an architecture issue here:
- the API you introduced in Core only emit signals, so that's not something a user can use from a script
Yes. This was the wrong way to do it indeed. I have introduced a 'setFindWidget(...)' which function which is called from the MainWindow.
Fixed. Thank you. - HighlightSearchDelegate shouldn't be in Core, but gui only <-- Fixed. Thank you.
IMO there shouldn't be any specific changes into Core, only gui. So we need a way for the FindWidget to work on views instead of working on document. Meaning you need some sort of interfaces for the searchable views to call from the FinWidget. Fixed.
Concerning the rc views it is a bit different as we have 3 different views to handle for the search.
So each view has its own search widgets.
I like the idea of the highlight delegate, but don't make the color blue (it's hardly readable for me), maybe use bold and/or underline. <--- Fixed (using bold - looks better indeed). Thank you.
I'm not sure we want to do replace on ts and ui view yet, so maybe have a way to hide this part for now when on the ts or ui view. Fixed (I saved the find and replace code locally in case we want to add this feature at some point) .
Last thing, when I press escape and hide the find widget, I would expect to reset the search delegate.
Yep - did not notice that - fixed. Thank you.
5a51860
to
15b61a5
Compare
Comments fixed. Ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, don't do anything on the rcfileview.
In term of usability, I would like to use the same find widget as the rest, I need to figure out how to do that though. So let's remove that out.
Fixes tasks: QtUiDocument QtTsDocument RCDocument text view RCDocument central view RCDocument left tree
15b61a5
to
a9473c3
Compare
Comments fixed. Ready for review. |
I did some changes, but apparently I can't edit this PR. |
strings.h
intostring_helper.h
array<>
in docDocument
andProject
APIProject
#143)