From ba1764ffc2cdc36003adfa2ededd8b94f5df3514 Mon Sep 17 00:00:00 2001 From: hy2k <17329720+hy2k@users.noreply.github.com> Date: Mon, 8 Apr 2024 03:01:48 +0900 Subject: [PATCH] Patch for Search Filtering Feature (#237) # Problem This PR is a continuation of the work done in PR #231. It aims to address the incomplete fix I implemented in my previous PR. The problem addressed by this PR pertains to the handling of 'story' nodes in our component tree. In the current implementation, 'story' nodes, which inherently do not have any children, are incorrectly subjected to a descendants check. This results in the `isEmpty` flag always being set to true for 'Story' nodes. # Solution The main changes in this PR include: - Refactoring the search filtering logic into a separate module. - Adding unit tests While the changes introduced in this PR could have been implemented within the existing component, I chose to refactor them into a separate module, so it's easier to test. # Checklist - [x] Ran `./bin/test.sh` locally - [x] Ran `./bin/analyze.sh` locally --------- Co-authored-by: Marin Minnerly --- src/Explorer/Component/init.lua | 20 +---- src/Explorer/filterComponentTreeNode.lua | 27 +++++++ src/Explorer/filterComponentTreeNode.spec.lua | 77 +++++++++++++++++++ src/Explorer/types.lua | 2 +- 4 files changed, 108 insertions(+), 18 deletions(-) create mode 100644 src/Explorer/filterComponentTreeNode.lua create mode 100644 src/Explorer/filterComponentTreeNode.spec.lua diff --git a/src/Explorer/Component/init.lua b/src/Explorer/Component/init.lua index b85b5fbf..03e7a27d 100644 --- a/src/Explorer/Component/init.lua +++ b/src/Explorer/Component/init.lua @@ -5,7 +5,7 @@ local Sift = require(flipbook.Packages.Sift) local Directory = require(script.Directory) local Story = require(script.Story) local types = require(flipbook.Explorer.types) -local getTreeDescendants = require(flipbook.Explorer.getTreeDescendants) +local filterComponentTreeNode = require(flipbook.Explorer.filterComponentTreeNode) local e = React.createElement @@ -60,22 +60,8 @@ local function Component(providedProps: Props) end end - if props.filter then - if props.node.icon == "story" and not props.node.name:lower():match(props.filter:lower()) then - return - end - - local isEmpty = true - for _, descendant in getTreeDescendants(props.node) do - if descendant.name:lower():match(props.filter:lower()) then - isEmpty = false - break - end - end - - if isEmpty then - return - end + if props.filter and filterComponentTreeNode(props.node, props.filter) then + return end return e("Frame", { diff --git a/src/Explorer/filterComponentTreeNode.lua b/src/Explorer/filterComponentTreeNode.lua new file mode 100644 index 00000000..ff3bbc2a --- /dev/null +++ b/src/Explorer/filterComponentTreeNode.lua @@ -0,0 +1,27 @@ +local flipbook = script:FindFirstAncestor("flipbook") + +local types = require(flipbook.Explorer.types) + +local getTreeDescendants = require(script.Parent.getTreeDescendants) + +local function filterComponentTreeNode(node: types.ComponentTreeNode, filter: string): boolean + if node.icon == "story" then + if not node.name:lower():match(filter:lower()) then + return true + end + + return false + end + + local isEmpty = true + for _, descendant in getTreeDescendants(node) do + if descendant.name:lower():match(filter:lower()) then + isEmpty = false + break + end + end + + return isEmpty +end + +return filterComponentTreeNode diff --git a/src/Explorer/filterComponentTreeNode.spec.lua b/src/Explorer/filterComponentTreeNode.spec.lua new file mode 100644 index 00000000..d8edd6e2 --- /dev/null +++ b/src/Explorer/filterComponentTreeNode.spec.lua @@ -0,0 +1,77 @@ +local flipbook = script:FindFirstAncestor("flipbook") + +local types = require(flipbook.Explorer.types) + +return function() + local filterComponentTreeNode = require(script.Parent.filterComponentTreeNode) + + it("should return true when the query does not match the story name", function() + local target: types.ComponentTreeNode = { + children = {}, + name = "test", + icon = "story", + } + local query = "other" + + local result = filterComponentTreeNode(target, query) + expect(result).to.equal(true) + end) + + it("should return false the query matches the story name", function() + local target: types.ComponentTreeNode = { + children = {}, + name = "test", + icon = "story", + } + local query = "tes" + + local result = filterComponentTreeNode(target, query) + expect(result).to.equal(false) + end) + + it("should return true when the filter does not match any of node in tree", function() + local target: types.ComponentTreeNode = { + children = { + { + children = {}, + name = "test", + icon = "story", + }, + { + children = {}, + name = "folder", + icon = "folder", + }, + }, + name = "storybook", + icon = "storybook", + } + local query = "other" + + local result = filterComponentTreeNode(target, query) + expect(result).to.equal(true) + end) + + it("should return false when a filter match at least one of nodes in tree", function() + local target: types.ComponentTreeNode = { + children = { + { + children = {}, + name = "test", + icon = "story", + }, + { + children = {}, + name = "folder", + icon = "folder", + }, + }, + name = "storybook", + icon = "storybook", + } + local query = "tes" + + local result = filterComponentTreeNode(target, query) + expect(result).to.equal(false) + end) +end diff --git a/src/Explorer/types.lua b/src/Explorer/types.lua index 9f22ebe7..706f7ae4 100644 --- a/src/Explorer/types.lua +++ b/src/Explorer/types.lua @@ -6,7 +6,7 @@ type Storybook = storybookTypes.Storybook export type ComponentTreeNode = { name: string, children: { ComponentTreeNode }, - icon: string?, + icon: ("folder" | "story" | "storybook")?, instance: Instance?, storybook: Storybook?, }