[Update] Add category to sample row

[CalebRas]

Description

This PR adds the category to the sample row in the Sample Viewer. This allows the user to easily find the category of a sample when searching by hitting the description button on the sample row. The description button was also updated from an info symbol to a chevron due to a discussion that can be found in the issue's comments.

Linked Issue(s)

• swift/issues/4298

How To Test

• Search for a sample.
• Tap on the chevron to see the sample's category and description.
• Tap on the chevron again to close the description.
• Tap on the sample row to open the sample.

Screenshots

Before	After

[yo1995]

After tweaking for quite a while, I think I found the shortest code that is the closest to what I imagined. (and am quite proud that I didn't give up 😊 )

• Rotating chevron with non-glitching animation
• No hack to hide the navigation link gray chevron
• Use more native SwiftUI APIs

Even better, it gets rid of the boolean state in the row!

Can you try the code below on all targets, and see if they all behave correctly? Thanks.

Some readings I did

• https://developer.apple.com/documentation/swiftui/disclosuregroup
• https://stackoverflow.com/questions/70396915/how-to-do-a-reveal-style-collapse-expand-animation-in-swiftui
• https://www.appcoda.com/hide-disclosure-indicator-swiftui-list/
• https://gist.github.com/kurtjacobsdev/61b360cd1c86b72cf5f35a6a3ec1ffdb
• https://kristaps.me/blog/swiftui-disclosure-group/

import SwiftUI

struct SampleListView: View {
    /// All samples that will be displayed in the list.
    let samples: [Sample]
    
    /// A Boolean value indicating whether the row description should include
    /// the sample's category. We only need this information when the samples
    /// in the list are from different categories.
    let shouldShowCategory: Bool // can be passed in from parent view, or computed in place such as Set(samples.map(\.category)).count > 1
    
    var body: some View {
        List(samples, id: \.name) { sample in
            SampleRow(sample: sample, shouldShowCategory: shouldShowCategory)
        }
    }
}

private extension SampleListView {
    struct SampleRow: View {
        /// The sample displayed in the row.
        let sample: Sample
        
        /// A Boolean value that indicates whether to show the sample's category.
        let shouldShowCategory: Bool

        var body: some View {
            DisclosureGroup {
                VStack(alignment: .leading) {
                    if shouldShowCategory {
                        Text("Category: \(sample.category)")
                            .bold()
                    }
                    Text(sample.description)
                        .foregroundColor(.secondary)
                }
                .listRowSeparator(.hidden)
                .font(.caption)
            } label: {
                NavigationLink(sample.name) {
                    SampleDetailView(sample: sample)
                }
            }
        }
    }
}

[CalebRas]

Looks awesome, nice job! The only thing I found is the chevron appears on the other side of the sample name in Mac Catalyst. I am not sure if that is expected behavior or not, though.

[yo1995]

Let's see what others think

[des12437]

I'm okay with closing #170 in favor of using DisclosureGroup. It looks good on iOS and iPad, but I notice some issues on Mac Catalyst. The alignment and spacing of the icons is off and the button's selection color is purple when selecting the button twice. When the selection color is purple then the icon is not visible. Even with these issues on Mac Catalyst, DisclosureGroup is more favorable than the other implementation.

[dfeinzimer]

General question: Have we considered displaying the description by default with no control over showing/hiding it?

Subtitles/captions/descriptions under list items seem to be a common pattern throughout the OS (recent call entries in Phone, Notes, Messages, Reminders, etc..).

[CalebRas]

General question: Have we considered displaying the description by default with no control over showing/hiding it?

Subtitles/captions/descriptions under list items seem to be a common pattern throughout the OS (recent call entries in Phone, Notes, Messages, Reminders, etc..).

Yea, Ting and I have talked about it. I think it would clutter the searching view too much. The difference between the Sample View and apps like Messages is we are displaying two things, the category and the description, while they are only displaying one. Also, some of the descriptions can be three lines long, which is a lot to display. @dfeinzimer

[yo1995]

General question: Have we considered displaying the description by default with no control over showing/hiding it?

It would probably make sense to limit the description display to numberOfLines = 2. Messages and Mails app both have a fixed height row, and it truncates the text with ellipsis if it is too long. We can do that as well, but I feel it might diminish the value of the description.

[dfeinzimer]

It would probably make sense to limit the description display to numberOfLines = 2. Messages and Mails app both have a fixed height row, and it truncates the text with ellipsis if it is too long. We can do that as well, but I feel it might diminish the value of the description.

Another idea I have would be to swap the DisclosureGroup for "More" and "Less" buttons under the description. Initially limit the description at two lines, and if there're more lines to be shown, show a "More" button that removes the line limit and a "Less" button that re-applies the line limit.

[rolson]

Another idea I have would be to swap the DisclosureGroup for "More" and "Less" buttons under the description. Initially limit the description at two lines, and if there're more lines to be shown, show a "More" button that removes the line limit and a "Less" button that re-applies the line limit.

I proposed the same thing in a different thread.

[CalebRas]

It would probably make sense to limit the description display to numberOfLines = 2. Messages and Mails app both have a fixed height row, and it truncates the text with ellipsis if it is too long. We can do that as well, but I feel it might diminish the value of the description.

Another idea I have would be to swap the DisclosureGroup for "More" and "Less" buttons under the description. Initially limit the description at two lines, and if there're more lines to be shown, show a "More" button that removes the line limit and a "Less" button that re-applies the line limit.

Looking through Apple's apps, such as the Mail app, I have found the DisclosureGroup used more than "More"/"Less" pattern, especially when dealing with lists. List rows are generally a fixed size, so I can't think of any examples where a row's height dynamically grows a with "More"/"Less" button. Did you have any apps you were thinking of?

[dfeinzimer]

Did you have any apps you were thinking of?

I know I've seen it (not necessarily in a list view) but I can't recall where. It may have been a 3rd party non-Apple app.

I did some searching around the HIG and found something that seems relevant:

Use an info button only to reveal more information about a row’s content. An info button — called a detail disclosure button when it appears in a list row — doesn’t support navigation through a hierarchical table or list. If you need to let people drill into a list or table row’s subviews, use a disclosure indicator accessory control.

Granted this doesn't depict items in a sidebar but nonetheless seems to recommend against using disclosures to reveal info in lists.

[mhdostal]

A disclosure indicator reveals the next level in a hierarchy; it doesn't show details about the item.

The joy of UI design: this bit in the HIG seems to contradict the above:

Disclosure triangles
A disclosure triangle shows and hides information and functionality associated with a view or a list of items.

I'm OK with the Disclosure Triangle. It seems more appropriate than the "info" button.

[philium]

I had a thought. Instead of putting the category in each row, we could group the search results by category, putting all of the results for each category in a different section with the category name as the section header.

[CalebRas]

I had a thought. Instead of putting the category in each row, we could group the search results by category, putting all of the results for each category in a different section with the category name as the section header.

I am not sure that would work with the search bar improvements PR that I have up. #228 breaks up the search results into three sections, Name, Description, and Tags, based on how the results were found. Were you hoping to get rid of that design in favor of category sections? @philium

[philium]

Were you hoping to get rid of that design in favor of category sections?

I hand't seen that other design. But if the search result row shows all three things (the title, the description, and the tags), then they could still be grouped into sections by category. Rows could be similar to the Reminders app, which shows a title, notes, and tags:

I'm not saying make it look exactly the same, but have those same elements in that order.

[yo1995]

Initially I asked Caleb to add the category information to the search result rows for the following reason: in the old app, when I search a sample by name, I sometimes cannot figure out which category it belongs to (especially for Maps and Layers samples). Next time when I want to find the sample again, I have to search again, or browse through all the categories.

With the addition of this information, now I can nail down the category more quickly with the first search. And next time I don't need to search again.

As you may tell, the category info is quite trivial, thus I suggested to only include it in the search results, and hide it in a disclose-able view. Putting them into separate sections might make this info too prominent; displaying the category and the description by default might also put too much visual saliency on these info with less importance.

---

Apart from the category info, other changes look good.

[CalebRas]

After discussion with Divesh, we determined that adding the category row is not needed because it would not be of much use to the users. Updating the info button will happen under a different issue/PR. Thank you to everyone that gave feedback on this issue.