[PR #3109] [MERGED] feat: selectOption sort + fix logic in edge cases #5572

New issue

Closed

opened 2026-03-23 22:19:20 +00:00 by mirror · 0 comments

mirror commented

2026-03-23 22:19:20 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/AppFlowy-IO/AppFlowy/pull/3109
Author: @richardshiue
Created: 8/3/2023
Status: ✅ Merged
Merged: 8/9/2023
Merged by: @appflowy

Base: main ← Head: empty-cell-sort-fix-and-test

📝 Commits (10+)

cc25842 fix: multiple sorts involving empty checkbox cells
8026648 test: add test to cover sorting with empty cells
bdac3a6 chore: remove deleted module
ad1a6d4 test: fix tests
c3909db fix: sort logic
8cee2fb feat: support select option sort
66087d6 style: fix lint
7571377 fix: filter tests and actually enable sort
0dc8ebd fix: share export test
c947a50 chore: expand generic logic to trait

📊 Changes

37 files changed (+522 additions, -307 deletions)

View changed files

📝 frontend/appflowy_flutter/lib/plugins/database_view/application/field/field_controller.dart (+2 -0)
📝 frontend/rust-lib/flowy-core/src/module.rs (+1 -1)
📝 frontend/rust-lib/flowy-database2/src/services/database_view/layout_deps.rs (+2 -3)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/checkbox_type_option/checkbox_type_option.rs (+28 -11)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/checkbox_type_option/checkbox_type_option_entities.rs (+7 -1)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist.rs (+5 -7)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist_entities.rs (+7 -1)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_tests.rs (+2 -1)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_type_option.rs (+10 -9)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_type_option_entities.rs (+7 -1)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/number_type_option/number_type_option.rs (+35 -14)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/multi_select_type_option.rs (+39 -27)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_ids.rs (+7 -1)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs (+3 -2)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/single_select_type_option.rs (+12 -11)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/text_type_option/text_type_option.rs (+19 -7)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option.rs (+35 -4)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option_cell.rs (+55 -26)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option.rs (+15 -7)
📝 frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option_entities.rs (+7 -1)

...and 17 more files

📄 Description

This PR fixes the sort logic in multi-sort scenarios.

The following logic is in place, hopefully it makes sense. Here cell means the cell of the row that is under the field being used to sort the grid and sort condition is either ascending or descending

2 uninitialized cells should preserve natural order
uninitialized cell and initialized cell that is regarded as empty (empty text string, empty date timestamp, empty list of select ids, etc) should preserve natural order
BUT if the field type is checkbox then the uninitialized cell must be regarded as UNCHECKED and compared to the initialized cell according to the sort condition
a pair of uninitialized and initialized cell that is not empty should move the initialized cell to the top regardless of sort condition
2 initialized that are both regarded as empty should preserve natural order
2 initialized cells that are both not empty should be compared against each other according to the field type and sort condition
1 initialized cell that is regarded as empty and another initialized cell that is not empty should move the non-empty cell to the top regardless of sort condition

Feature Preview

fix: #3108

PR Checklist

My code adheres to the AppFlowy Style Guide
I've listed at least one issue that this PR fixes in the description above.
I've added a test(s) to validate changes in this PR, or this PR only contains semantic changes.
All existing tests are passing.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/AppFlowy-IO/AppFlowy/pull/3109 **Author:** [@richardshiue](https://github.com/richardshiue) **Created:** 8/3/2023 **Status:** ✅ Merged **Merged:** 8/9/2023 **Merged by:** [@appflowy](https://github.com/appflowy) **Base:** `main` ← **Head:** `empty-cell-sort-fix-and-test` --- ### 📝 Commits (10+) - [`cc25842`](https://github.com/AppFlowy-IO/AppFlowy/commit/cc2584284f64e3ded69d74bc0412b07a9dbf5042) fix: multiple sorts involving empty checkbox cells - [`8026648`](https://github.com/AppFlowy-IO/AppFlowy/commit/8026648fbf63e6f9255db0adc2bf4f2d153e8722) test: add test to cover sorting with empty cells - [`bdac3a6`](https://github.com/AppFlowy-IO/AppFlowy/commit/bdac3a6a847b92a13e775c9a19e0b0f186ad1c79) chore: remove deleted module - [`ad1a6d4`](https://github.com/AppFlowy-IO/AppFlowy/commit/ad1a6d401b9405c4d253e1a789e521c492d556cc) test: fix tests - [`c3909db`](https://github.com/AppFlowy-IO/AppFlowy/commit/c3909db12863269c9f26449a39918f69cf52699b) fix: sort logic - [`8cee2fb`](https://github.com/AppFlowy-IO/AppFlowy/commit/8cee2fbcd52b788f5787e01c9036614e0eb2be3a) feat: support select option sort - [`66087d6`](https://github.com/AppFlowy-IO/AppFlowy/commit/66087d6def8ddd424b4580b790c74b8060339b37) style: fix lint - [`7571377`](https://github.com/AppFlowy-IO/AppFlowy/commit/7571377058c838e2e8f1269e17c906d8786f2e25) fix: filter tests and actually enable sort - [`0dc8ebd`](https://github.com/AppFlowy-IO/AppFlowy/commit/0dc8ebd412ca755d764cddf64c8810914e67290c) fix: share export test - [`c947a50`](https://github.com/AppFlowy-IO/AppFlowy/commit/c947a508659053db2d56d192b85f4d4314c859cf) chore: expand generic logic to trait ### 📊 Changes **37 files changed** (+522 additions, -307 deletions) <details> <summary>View changed files</summary> 📝 `frontend/appflowy_flutter/lib/plugins/database_view/application/field/field_controller.dart` (+2 -0) 📝 `frontend/rust-lib/flowy-core/src/module.rs` (+1 -1) 📝 `frontend/rust-lib/flowy-database2/src/services/database_view/layout_deps.rs` (+2 -3) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/checkbox_type_option/checkbox_type_option.rs` (+28 -11) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/checkbox_type_option/checkbox_type_option_entities.rs` (+7 -1) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist.rs` (+5 -7) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist_entities.rs` (+7 -1) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_tests.rs` (+2 -1) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_type_option.rs` (+10 -9) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_type_option_entities.rs` (+7 -1) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/number_type_option/number_type_option.rs` (+35 -14) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/multi_select_type_option.rs` (+39 -27) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_ids.rs` (+7 -1) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs` (+3 -2) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/single_select_type_option.rs` (+12 -11) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/text_type_option/text_type_option.rs` (+19 -7) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option.rs` (+35 -4) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option_cell.rs` (+55 -26) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option.rs` (+15 -7) 📝 `frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option_entities.rs` (+7 -1) _...and 17 more files_ </details> ### 📄 Description This PR fixes the sort logic in multi-sort scenarios. The following logic is in place, hopefully it makes sense. Here `cell` means the cell of the row that is under the field being used to sort the grid and sort condition is either ascending or descending - 2 uninitialized cells should preserve natural order - uninitialized cell and initialized cell that is regarded as empty (empty text string, empty date timestamp, empty list of select ids, etc) should preserve natural order - BUT if the field type is checkbox then the uninitialized cell must be regarded as UNCHECKED and compared to the initialized cell according to the sort condition - a pair of uninitialized and initialized cell that is not empty should move the initialized cell to the top regardless of sort condition - 2 initialized that are both regarded as empty should preserve natural order - 2 initialized cells that are both not empty should be compared against each other according to the field type and sort condition - 1 initialized cell that is regarded as empty and another initialized cell that is not empty should move the non-empty cell to the top regardless of sort condition ### Feature Preview fix: #3108 #### PR Checklist - [x] My code adheres to the [AppFlowy Style Guide](https://appflowy.gitbook.io/docs/essential-documentation/contribute-to-appflowy/software-contributions/submitting-code/style-guides) - [x] I've listed at least one issue that this PR fixes in the description above. - [x] I've added a test(s) to validate changes in this PR, or this PR only contains semantic changes. - [ ] All existing tests are passing. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>