feat: treeview component #421

LinaYahya · 2023-11-29T13:07:04Z

closes #413

pyphilia

Thanks for your PR! 🍃
So what do you do for the homepage? Is it using the old version? I don't remember.
I confused this PR with the builder's one, so maybe some comments are irrelevant but let me know.

cypress/e2e/navigation.cy.ts

cypress/e2e/ws.cy.ts

cypress/support/integrationUtils.ts

package.json

src/modules/tree/style.css

src/modules/tree/DynamicTreeView.tsx

src/utils/item.ts

pyphilia

Thanks for your work, I think it still need some refactor. Try to add more comments as well because it takes some time to understand why you're doing some things ☃️

src/modules/item/ItemNavigation.tsx

src/modules/tree/TreeView.tsx

src/utils/item.ts

pyphilia · 2023-12-20T16:42:42Z

src/utils/item.ts

+  if (data.length === 1) {
+    tree[data[0].id] = { ...data[0], children: [] };
+  }


Do you need a special case for 1? Won't it work with the normal process?

What about this?

basically, we build our tree based on parent-children relation which will not work for a non-children Item, As we build a map tree based on parent

pyphilia · 2023-12-20T16:45:07Z

src/utils/item.ts

+  const keys = Object.keys(mapTree);
+  const allChildren: string[] = keys.reduce((acc: string[], key: string) => {
+    const node = mapTree[key as keyof Tree];
+    if (node && node.children) {
+      acc.push(...node.children);
+    }
+    return acc;
+  }, []);
+  const rootKeys = keys.filter((key) => !allChildren.includes(key));


Why do you need to find the rootKeys? I think for a normal player view you only have one root. Can't you define it in the arguments? Let's not take into account the home view.

We need to get the first parent, this is the only way to get it even it's only one root

pyphilia

Some issues:

There is a random "show more" when I open any item.

There is a unwanted "flickering" when:

You click on a folder A. It expands as expected.
You click on another folder B. It expands as expected.
You click on A again, the folder folds. I think we should avoid this folding.
My previous way of solving that was to only deal with expanding/folding using the arrow but no one liked it.
Do you think it's possible to do something like:
click on folder A show and expands A.
click on folder B shows and expands B.
click on folder A again does not folds it but show it's content.
clicking on the arrow of A folds/expands it but does not show the content.

src/utils/item.ts

pyphilia · 2023-12-22T12:07:12Z

src/utils/item.ts

+  if (data.length === 1) {
+    tree[data[0].id] = { ...data[0], children: [] };
+  }


What about this?

src/utils/item.ts

spaenleh

I proposed some changes in my commit to improve typings and also support the file mimetype icons.

I also tries to understand why the WS test was failing, see my explanation in the review. I add it here for visibility

The unexpected error that you get when you run these tests locally is due to vite using Websockets for hot module replacement (HMR).
In this test we stub the Websockets, so when vite tries to use them, it fails. The easiest way to get around this and keep the test is to not use the development server for this test, but to use a staticaly built version of the website served by a basic webserver.
I.e.: Do not launch the player for the interactive tests with yarn start:test but use the build + preview commands: yarn build --mode test && yarn preview --port 3333 the port should match what cypress expects.
This means you will not be able to change the source code interactively for these tests, but at least they will not fail because of stupid Websockets mocking issues. Hope it helps !

I removed unnecessary waits in tests.

cypress/e2e/main.cy.ts

spaenleh · 2024-01-10T14:56:34Z

cypress/e2e/ws.cy.ts

@@ -8,6 +8,8 @@ import {
 import { FOLDER_WITH_SUBFOLDER_ITEM } from '../fixtures/items';
 import { expectFolderButtonLayout } from '../support/integrationUtils';

+Cypress.on('uncaught:exception', () => false);


@pyphilia @LinaYahya
The unexpected error that you get when you run these tests locally is due to vite using Websockets for hot module replacement (HMR).

In this test we stub the Websockets, so when vite tries to use them, it fails. The easiest way to get around this and keep the test is to not use the development server for this test, but to use a staticaly built version of the website served by a basic webserver.
I.e.: Do not launch the player for the interactive tests with yarn start:test but use the build + preview commands: yarn build --mode test && yarn preview --port 3333 the port should match what cypress expects.

This means you will not be able to change the source code interactively for these tests, but at least they will not fail because of stupid Websockets mocking issues. Hope it helps !

spaenleh · 2024-01-10T15:10:14Z

cypress/support/integrationUtils.ts

+  cy.wait(100);
+  menu.click();
  menu.get(`#${MAIN_MENU_ID}`).contains(name);


Clicking on the container is useless and results in not what is expected (opens the child that happens to be under it). I guess this was changed by @pyhpilia, maybe before the children were closed and clicking on the main menu was the only way to open it. Anyway in that case the chosen cypress selector should not have been MAIN_MENU as it will always be found (even when there is no data). Instead target something that is only present once data has been loaded (for example a selector with the item id of the root element).

…tems

LinaYahya changed the title ~~413 navigation bar~~ feat: treeview component Dec 1, 2023

LinaYahya requested a review from pyphilia December 1, 2023 14:01

pyphilia suggested changes Dec 7, 2023

View reviewed changes

LinaYahya self-assigned this Dec 15, 2023

LinaYahya requested a review from pyphilia December 18, 2023 10:00

LinaYahya temporarily deployed to development December 18, 2023 10:09 — with GitHub Actions Inactive

pyphilia suggested changes Dec 20, 2023

View reviewed changes

pyphilia temporarily deployed to development December 22, 2023 11:49 — with GitHub Actions Inactive

pyphilia suggested changes Dec 22, 2023

View reviewed changes

LinaYahya requested a review from pyphilia January 9, 2024 16:12

spaenleh added the improvement label Jan 10, 2024

pyphilia force-pushed the 413-navigation-bar branch from 23695f9 to 86c74e5 Compare January 10, 2024 10:58

pyphilia temporarily deployed to development January 10, 2024 13:11 — with GitHub Actions Inactive

pyphilia temporarily deployed to development January 10, 2024 14:04 — with GitHub Actions Inactive

pyphilia temporarily deployed to development January 10, 2024 14:11 — with GitHub Actions Inactive

spaenleh approved these changes Jan 10, 2024

View reviewed changes

spaenleh force-pushed the 413-navigation-bar branch from 90f4714 to c85f83f Compare January 10, 2024 18:51

spaenleh temporarily deployed to development January 10, 2024 18:52 — with GitHub Actions Inactive

spaenleh temporarily deployed to development January 11, 2024 07:07 — with GitHub Actions Inactive

LinaYahya added 11 commits January 11, 2024 10:21

feat: change tree component to react-accessible-treeview

83120b9

style: style selected item at navigation tree

38d502e

fix: failing tests due to navigation changes

d4e51d3

fix: rename component treeView and delete old component

a664dff

style: change active tab color

7b84690

build: refactor buildtree based on path not childrenOrder

aac8321

fix: type of node with children

edc2b18

fix: remove test navigation for home

8fef7e7

fix: remove mainItem props

1b93b7c

fix: treeview structure

bd3c816

fix: formatting

d27a4f1

LinaYahya and others added 9 commits January 11, 2024 10:22

build: refactor build treemap based on node not nodeId

ad91155

refactor: fix children order, change icons, use accessible items

791d158

test: fix tests

8ea9db4

refactor: fix folder without childrenorder, item type icon for root i…

784aa74

…tems

refactor: add extra to item icon

e80d431

test: remove useless wait and click

101561f

fix: proposed changes for PR

0de54ad

fix: remove only in test

c2f404a

refactor: update sdk

e3f47e7

pyphilia force-pushed the 413-navigation-bar branch from c85f83f to e3f47e7 Compare January 11, 2024 13:00

fix: update chatbox deps

f0e0fa7

pyphilia merged commit d6104ec into main Jan 11, 2024
2 checks passed

pyphilia deleted the 413-navigation-bar branch January 11, 2024 14:26

github-actions bot added the un-released label Jan 11, 2024

github-actions bot mentioned this pull request Jan 11, 2024

chore(main): release 2.3.0 #430

Merged

github-actions bot added v2.3.0 and removed un-released labels Jan 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: treeview component #421

feat: treeview component #421

LinaYahya commented Nov 29, 2023 •

edited

Loading

pyphilia left a comment

pyphilia left a comment

pyphilia Dec 20, 2023

pyphilia Dec 22, 2023

LinaYahya Jan 9, 2024

pyphilia Dec 20, 2023

LinaYahya Jan 9, 2024

pyphilia left a comment

pyphilia Dec 22, 2023

spaenleh left a comment •

edited

Loading

spaenleh Jan 10, 2024

spaenleh Jan 10, 2024

feat: treeview component #421

feat: treeview component #421

Conversation

LinaYahya commented Nov 29, 2023 • edited Loading

pyphilia left a comment

Choose a reason for hiding this comment

pyphilia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pyphilia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spaenleh left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LinaYahya commented Nov 29, 2023 •

edited

Loading

spaenleh left a comment •

edited

Loading