Settings Search
Description
This PR adds support for searching settings in CE's settings, it works very similar to the way System Settings searches.
Related Issues
- close #1309
Checklist
- [x] I read and understood the contributing guide as well as the code of conduct
- [x] The issues this PR addresses are related to each other
- [x] My changes generate no new warnings
- [x] My code builds and runs on my machine
- [x] My changes are all related to the related issue above
- [x] I documented my code
- [x] Code is clean
- [x] Highlighted text of search
- [x] Fix grouping issue
Screenshots
https://github.com/CodeEditApp/CodeEdit/assets/128280019/118ee588-9edb-4b73-a7f6-f70aa6f2d5e6
Extra notes
I've updated the docs for adding a new setting so that it appears in the search results
@luah5 :
https://github.com/CodeEditApp/CodeEdit/assets/20476002/022af962-8234-4c14-81fa-43fa78bb1537
- It would be possible for everyone in the same place to group them.
- When clicked, on the text it should open the place where it is which happens, but you don't understand where that text is, it would be possible to make it flash or something similar to how android does when you search for something in the search bar settings.
You'll notice that:
- The text is aligned with the parent item's label
- The child item itself is selectable
- There is only one parent item, in yours there is a parent for every child
- The search term is white while the rest of the text has an opacity (maybe
.secondaryLabelColor)
@luah5 : When selecting an element I don't think @austincondiff wants the whole group to be selected, at least macOS Ventura doesn't do that, it only selects the single element.
https://github.com/CodeEditApp/CodeEdit/assets/20476002/aa43b639-505a-456e-bed6-27508ca6d991
For point 2) I think it is useful to understand where the property is, then you need to hear what others think ask @austincondiff.
@Angelk90 I'd agree with out that it should probably flash or jump out at you in some way. System settings doesn't do this though so we might save this for another PR. Regardless, my guess is It is selecting all the related items because they all point to the same location, so they are all selected. We need to think of a workaround to this so only the item that is clicked is selected.
@luah5 : The thing that the whole group remains selected does not convince me.
https://github.com/CodeEditApp/CodeEdit/assets/20476002/0b80e01e-52a4-47e9-b53c-e92472fd17c9
@luah5 When there is no search text input, the menu item color needs to be primary. It looks like it is defaulting to secondary. It should only be secondary when there is text input into the search field.
@austincondiff , @luah5 : When the possibility of having CE in multiple languages will be enabled later and there will be all the translations, how will this system work at the moment?
Nice work! I noticed some issues:
- Tab selection state isn't kept
Could you show a video, does the tab selection need to be kept across sessions with UserDefaults?
- I should be able to go through the search results by pressing the up and down arrow keys
Re implementing that feature makes the search results group again
- It is difficult to search for settings because the search keywords don't match the "real" labels of the settings
Fixed.
This is a good base, but I think it needs improvements. I strongly suggest using settings labels instead of variable names as keywords. This is what will need to happen anyway. Not doing that now is a bit pointless, as the work from this PR would be removed later on. I would also like to avoid complaints from users about search not being accurate / predictable
Fixed.
@luah5 Several things:
- The text size is too small. Please use the original text size.
- The default color of main navigation items is too light.
- Result items text should line up with main items text. Give it a little more left spacing.
@austincondiff
luah5 Several things:
- The text size is too small. Please use the original text size.
Fixed, I'm not sure what happened though.
- The default color of main navigation items is too light.
New: Right, Old: Left
I can't see a difference, but if you were referring to the text, I have fixed that.
- Result items text should line up with main items text. Give it a little more left spacing.
Fixed in the latest commit
Font size and text color are still incorrect.
@luah5 :
https://github.com/CodeEditApp/CodeEdit/assets/20476002/1977dff2-2b49-4c52-8a92-30da7579e4b0
@luah5 Several things:
- The text size is too small. Please use the original text size.
- The default color of main navigation items is too light.
- Result items text should line up with main items text. Give it a little more left spacing.
- I just realized this is because of the sidebar size system setting. Disregard this.
- This looks like it was addressed. Thanks!
@luah5 :
Latest commit version, as you can see when I click on an element then it loses focus because the focus is on the input field.
Problem with font or size, looks weird.
mac os ventura.
https://github.com/CodeEditApp/CodeEdit/assets/20476002/71c3fcd7-84e9-48a4-a36a-591c1595f8b4
@luah5 :
Latest commit version, as you can see when I click on an element then it loses focus because the focus is on the input field.
Problem with font or size, looks weird.
mac os ventura.
Registrazione.schermo.2023-07-10.alle.15.50.34.mov
Fixed :)
@luah5 : Is it normal that it loses focus when I click?
https://github.com/CodeEditApp/CodeEdit/assets/20476002/81eae85e-7cb8-424b-b61f-fcee50311afd
@luah5 : Is it normal that it loses focus when I click?
https://github.com/CodeEditApp/CodeEdit/assets/20476002/81eae85e-7cb8-424b-b61f-fcee50311afd
That is not normal, and I cannot reproduce it, what version of macOs ventura are you on?
@luah5 : Ventura 13.2.1
I genuinely have no clue what's happening maybe a macOS bug. As you can see in this video the same issue doesn't occur on my machine, also check you are on the latest version.
https://github.com/CodeEditApp/CodeEdit/assets/128280019/16821687-9fcb-44b1-a65e-37bbed60f4b3
Thanks for the report @Angelk90, I appreciate it!
@austincondiff : Do you also have this problem?
Where are we on this PR? Is it ready to be approved? Is there anything pending revision?
@austincondiff, I'm just waiting for it to be reviewed.
What is the status on this PR? We need to either finish it or close it. There is a lot of good work here so I'd rather push it over the finish line if we can.
What is the status on this PR? We need to either finish it or close it. There is a lot of good work here so I'd rather push it over the finish line if we can.
Sorry for the recent inactivity, I'll try to get this finished with my limited time at the moment. Then this PR can be merged.
I would love to have this PR merged, could some people please review this PR?
@luah5 can you fix the conflict with CodeEdit.xcodeproj/project.pbxproj?
Then it should be fine for re-review.
@luah5 can you fix the conflict with
CodeEdit.xcodeproj/project.pbxproj? Then it should be fine for re-review.
sure
just fixed the merge conflicts, if a reviewer has time, please review this PR thanks!
@luah5 FYI, shorthand_operator should not trigger if you use Text() + Text().
see https://github.com/realm/SwiftLint/issues/4936, https://github.com/realm/SwiftLint/issues/4953 for more information.
In the interest of preserving the work that has been done and not letting this PR become stale, I merged this PR. For those that requested changes to this PR, feel free to open follow-up PR's for these changes.