Settings Search #1312

luah5 · 2023-06-12T20:38:33Z

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 🐞 Search system in settings still not working #1309

Checklist

I read and understood the contributing guide as well as the code of conduct
The issues this PR addresses are related to each other
My changes generate no new warnings
My code builds and runs on my machine
My changes are all related to the related issue above
I documented my code
Code is clean
Highlighted text of search
Fix grouping issue

Screenshots

shot.mov

Angelk90 · 2023-06-13T11:01:09Z

@luah5 :

Registrazione.schermo.2023-06-13.alle.12.55.26.mov

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.

austincondiff · 2023-06-13T15:28:21Z

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)

Angelk90 · 2023-06-13T19:51:26Z

@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.

Registrazione.schermo.2023-06-13.alle.21.45.14.mov

For point 2) I think it is useful to understand where the property is, then you need to hear what others think ask @austincondiff.

austincondiff · 2023-06-13T20:45:56Z

@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.

Angelk90 · 2023-06-15T09:21:12Z

@luah5 : The thing that the whole group remains selected does not convince me.

Registrazione.schermo.2023-06-15.alle.11.19.52.mov

CodeEdit/Utils/Extensions/String/String+camelCaseToProperWord.swift

Wouter01

Nice work! I noticed some issues:

Tab selection state isn't kept
I should be able to go through the search results by pressing the up and down arrow keys
It is difficult to search for settings because the search keywords don't match the "real" labels of the settings

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

Wouter01 · 2023-06-18T17:29:25Z

CodeEdit/Features/Settings/SettingsView.swift

+    private func populatePages() -> [SettingsPage] {
+        var pages: [SettingsPage] = []
+        let settingsData: SettingsData = SettingsData()
+
+        pages = createPageAndSettings(
+            settingsData.general,
+            parent: SettingsPage(.general, baseColor: .gray, icon: .system("gear")),
+            prePages: pages
+        )
+        pages = createPageAndSettings(
+            settingsData.accounts,
+            parent: SettingsPage(.accounts, baseColor: .blue, icon: .system("at")),
+            prePages: pages
+        )
+        pages = createPageAndSettings(
+            settingsData.theme,
+            parent: SettingsPage(.theme, baseColor: .pink, icon: .system("paintbrush.fill")),
+            prePages: pages
+        )
+        pages = createPageAndSettings(
+            settingsData.textEditing,
+            parent: SettingsPage(.textEditing, baseColor: .blue, icon: .system("pencil.line")),
+            prePages: pages
+        )
+        pages = createPageAndSettings(
+            settingsData.terminal,
+            parent: SettingsPage(.terminal, baseColor: .blue, icon: .system("terminal.fill")),
+            prePages: pages
+        )
+        pages = createPageAndSettings(
+            settingsData.sourceControl,
+            parent: SettingsPage(.sourceControl, baseColor: .blue, icon: .symbol("vault")),
+            prePages: pages
+        )
+        pages.append(SettingsPage(.location, baseColor: .green, icon: .system("externaldrive.fill")))
+
+        return pages
+    }


This is a very confusing pattern to build your pages. Consider letting the createPageAndSettings function return an array and combine all arrays afterwards

How would that look like in code?

austincondiff · 2023-06-18T20:20:20Z

@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.

Angelk90 · 2023-06-19T09:27:45Z

@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?

luah5 · 2023-06-19T16:52:54Z

So, I'm putting this PR back into Draft because as highlighted by @Wouter01's review comments something has to happen with naming. I'll post more in the discord, regarding what progress I'm making.

…ttings translation and add locationsSettings for better search results

fix merge conflicts

luah5 · 2023-06-24T15:46:13Z

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 added 2 commits June 12, 2023 20:45

add support for searching settings

d54c6a5

code cleanup and fixes

4f2284d

respond to some review comments from @austincondiff,

0aef181

luah5 and others added 2 commits June 14, 2023 21:04

highlight the containing letters/word

07a6dd9

Merge branch 'CodeEditApp:main' into feat/settings-search

6b0f0e6

taylorbyks reviewed Jun 16, 2023

View reviewed changes

CodeEdit/Utils/Extensions/String/String+camelCaseToProperWord.swift Outdated Show resolved Hide resolved

luah5 and others added 4 commits June 16, 2023 07:22

Merge branch 'CodeEditApp:main' into feat/settings-search

bb4d1dc

partially working "non grouping" settings search

e26d1fe

fully working non grouping settings.

3819eaa

fix swiftlint and remaining bugs

710c11e

luah5 requested a review from taylorbyks June 17, 2023 15:01

luah5 added 2 commits June 18, 2023 14:30

final fixes and documentation

ac0f789

bug fixes and remove swiftlint:disable

6c615cb

luah5 marked this pull request as ready for review June 18, 2023 14:11

luah5 added 2 commits June 18, 2023 15:58

remove uneeded code

d5e2cc6

rename file and make text primary when there is no search text

4f5fd82

Wouter01 requested changes Jun 18, 2023

View reviewed changes

0xWDG marked this pull request as draft June 19, 2023 19:04

luah5 and others added 4 commits June 24, 2023 08:41

partially respond to @wouter's comments by adding a dictionary for se…

c70e5ea

…ttings translation and add locationsSettings for better search results

changes

2b0a07c

Merge branch 'fix-issues' into merge-to-settings-search

be9c08f

fix merge conflict

ebb66b9

fix merge conflicts

luah5 added 2 commits June 24, 2023 14:38

fix swiftlint

e90c73b

final fix for swiftlint

61d5c55

luah5 requested a review from Wouter01 June 24, 2023 15:46

Settings Search #1312

Settings Search #1312

luah5 commented Jun 12, 2023 •

edited

Angelk90 commented Jun 13, 2023

austincondiff commented Jun 13, 2023 •

edited

Angelk90 commented Jun 13, 2023

austincondiff commented Jun 13, 2023

Angelk90 commented Jun 15, 2023

Wouter01 left a comment •

edited

Wouter01 Jun 18, 2023

luah5 Jun 24, 2023

austincondiff commented Jun 18, 2023

Angelk90 commented Jun 19, 2023

luah5 commented Jun 19, 2023

luah5 commented Jun 24, 2023

Settings Search #1312

Are you sure you want to change the base?

Settings Search #1312

Conversation

luah5 commented Jun 12, 2023 • edited

Description

Related Issues

Checklist

Screenshots

Angelk90 commented Jun 13, 2023

austincondiff commented Jun 13, 2023 • edited

Angelk90 commented Jun 13, 2023

austincondiff commented Jun 13, 2023

Angelk90 commented Jun 15, 2023

Wouter01 left a comment • edited

Choose a reason for hiding this comment

Wouter01 Jun 18, 2023

Choose a reason for hiding this comment

luah5 Jun 24, 2023

Choose a reason for hiding this comment

austincondiff commented Jun 18, 2023

Angelk90 commented Jun 19, 2023

luah5 commented Jun 19, 2023

luah5 commented Jun 24, 2023

luah5 commented Jun 12, 2023 •

edited

austincondiff commented Jun 13, 2023 •

edited

Wouter01 left a comment •

edited