Skip to content

Commit

Permalink
Patch for Search Filtering Feature (#237)
Browse files Browse the repository at this point in the history
# 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 <[email protected]>
  • Loading branch information
hy2k and vocksel authored Apr 7, 2024
1 parent cc6e6b7 commit ba1764f
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 18 deletions.
20 changes: 3 additions & 17 deletions src/Explorer/Component/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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", {
Expand Down
27 changes: 27 additions & 0 deletions src/Explorer/filterComponentTreeNode.lua
Original file line number Diff line number Diff line change
@@ -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
77 changes: 77 additions & 0 deletions src/Explorer/filterComponentTreeNode.spec.lua
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion src/Explorer/types.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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?,
}
Expand Down

0 comments on commit ba1764f

Please sign in to comment.