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

Sorted fields from values.schema.json #230

Merged
merged 4 commits into from
May 16, 2024

Conversation

RohanMishra315
Copy link
Contributor

Issue: #228

  1. Sorted the fields from values.schema.json into the alphabetical form when the order is not provided
  2. When order is provided , the defined fields will come in order first and then the sorted fields.

Issue: cyclops-ui#228

1. Sorted the fields from values.schema.json into the alphabetical form when the order is not provided
2.  When order is provided , the defined fields will come in order first and then the sorted fields.
@RohanMishra315 RohanMishra315 requested a review from a team as a code owner April 27, 2024 15:39
Copy link
Collaborator

@petar-cvit petar-cvit left a comment

Choose a reason for hiding this comment

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

Thanks @RohanMishra315 for the PR. I left a comment on the sorting logic

Comment on lines 96 to 107
// Custom order provided, sort fields based on the order
ordersMap := make(map[string]int)

// Map field names to their indices in the custom order
for i, s := range order {
ordersMap[s] = i
}

// Sort fields based on their indices in the custom order
sort.Slice(fields, func(i, j int) bool {
return ordersMap[fields[i].Name] < ordersMap[fields[j].Name]
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your solution works when the order is provided for all fields or not provided at all, but it doesn't work when 3/6 fields have the order defined. You can check the issue for further explanation of such a case.
I tried it with the following template, and I got the order from the picture.

repo: https://github.com/cyclops-ui/templates
path: demo-extended
version: field-alphabetical-sort
Screenshot 2024-04-28 at 19 07 10

@RohanMishra315
Copy link
Contributor Author

RohanMishra315 commented May 13, 2024

    var orderedFields []models.Field
    var unorderedFields []models.Field

    for _, field := range fields {
        if idx, ok := ordersMap[field.Name]; ok {
            orderedFields = append(orderedFields, models.FieldWithOrder{Field: field, OrderIndex: idx})
        } else {
            unorderedFields = append(unorderedFields, field)
        }
    }

    // Sort fields with order based on the order index
    sort.Slice(orderedFields, func(i, j int) bool {
        return orderedFields[i].OrderIndex < orderedFields[j].OrderIndex
    })

    // Reconstruct the sorted fields slice
    sortedFields := make([]models.Field, 0, len(fields))
    for _, field := range orderedFields {
        sortedFields = append(sortedFields, field.Field)
    }
    sortedFields = append(sortedFields, unorderedFields...)

    fields = sortedFields
}

return fields

}
I think this would work I first separate the fields into two groups: those with a defined order and those without. We then sort the fields with a defined order based on the provided order index. Finally, we reconstruct the sorted fields slice by appending the ordered fields followed by the unordered fields. This approach ensures that fields with a defined order are prioritized and appear before the alphabetically sorted ones.

@hanshal101
Copy link
Contributor

Hey can you edit the comment again I think there's some mistake

@petar-cvit
Copy link
Collaborator

Sounds good to me. The only thing I wouldn't do is introduce a new struct, FieldWithOrder. Instead, you can still sort them like this:

sort.Slice(fields, func(i, j int) bool {
	return ordersMap[fields[i].Name] < ordersMap[fields[j].Name]
})

@petar-cvit
Copy link
Collaborator

@RohanMishra315 feel free to update the PR with those changes

Updated the sorting fields
Comment on lines 75 to 95
if len(order) == 0 {
// Extract names of fields for sorting
fieldNames := make([]string, len(fields))
for i, field := range fields {
fieldNames[i] = field.Name
}

// Sort field names alphabetically
sort.Strings(fieldNames)

// Map field names to their indices in the sorted array
fieldIndices := make(map[string]int)
for i, name := range fieldNames {
fieldIndices[name] = i
}

// Use the sorted indices to sort the fields
sort.Slice(fields, func(i, j int) bool {
return fieldIndices[fields[i].Name] < fieldIndices[fields[j].Name]
})
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You dont need this part. The code in the else statement should handle it.

Comment on lines 107 to 110
}

return fields
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

You closed the function and returned after closing it

Copy link
Collaborator

@petar-cvit petar-cvit left a comment

Choose a reason for hiding this comment

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

Thanks @RohanMishra315!

@petar-cvit petar-cvit merged commit fff607c into cyclops-ui:main May 16, 2024
1 check passed
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.

None yet

3 participants