#23 Left static sidebar #5

Salam-Dalloul · 2024-05-04T17:50:44Z

- static left sidebar with 3 buttons - first button to compress the sidebar and leave only the minimal part visible

aranega

It looks good to me, but it might be interesting to see if we can use some new concepts from react (<Suspens>). If we cannot, there is no problem.

The request for change comes from the fact that the PR is going to CELE-22 which is already merged, please redirect it towards develop and check/fix the merge conflicts if any ☺️

aranega · 2024-05-13T15:53:03Z

applications/visualizer/frontend/src/App.tsx


-                    </Box>
+  const hasLaunched = currentWorkspaceId != undefined;


I'm not 100% sure as I'm not very skilled for everything frontend related 😅, but instead of having the hasLaunched variable and the classical ternary operator, could it be one of those scenarios where we could use <Suspense></Suspense>?

aranega · 2024-05-13T19:22:41Z

applications/visualizer/frontend/src/components/Workspace.tsx

-            </ThemeProvider>
-        </>
-    )
+  const isLoading = LayoutComponent === undefined;


Same here, would it be interesting to see if we can use Suspens?

@aranega for this one we can use the Suspens as there is a loading state until we get the data, but for the previous one it's more a conditional rendering between 2 components that we have, I only used it here as it's more suitable

@Salam-Dalloul Ok, got it! Thanks for the clarification

@Salam-Dalloul I can see that at the end the isLoading is staying. I tried to remove it, but there is an issue, something is not loading well after. I guess that if we cannot remove it, then we can remove the use of <Suspense> all the point was to not use the isLoading variable :)

fyi @aranega the crashing was because the initial state of the LayoutComponent was undefined, there is a possible solution to put the initial state as a Loading component

const [LayoutComponent, setLayoutComponent] = useState<React.ComponentType>(() => LoadingComponent);

this will fix the crashing issue and will show the loading as an initial state until we have the LayoutComponent without the need of isLoading, please let me know if there is a better way, and thanks for the suggestion :)

vidhya-metacell

Hey @Salam-Dalloul
I was reading the details for the issue CELE/23, according to the tasks, i find the following points missing( I am not sure if there was any discussion to not do the below points)

Search and Add from neurons, the dropdown is not like figma yet its empty( also change the placeholder value for the input)
bin icon to remove neuron
- to create groups that makes a modal with input text for the group name appearing
- per neuron to add neuron to a group through the menu linked to it
Datasets -dropdown with the list of all categories (filter as well).

For the UI

Height should be 56 as in figma
WIdth is 56 in figma
Border radius is 6 in figma
Width is 300px in figma
Yellow margin should be 12. not 24
Scrollbar is not functional
Text truncation is not working
Space between the development stage 1 heading the the items below is incorrect

9)Height in figma is 52

Margin top is wrong
Hover state for dataset option is missing

In figma its more of a rectangle than a circle

Tooltip for the switches is missing

aranega

@Salam-Dalloul The modifications you made for my side are nice :), thanks. Just perhaps remove the <Suspense> as the isLoading variable is still needed.

The build step is failing currently due to multiple small issues:

unused imports => better to remove them
unused variable => some are for constants, if they are not used right now, perhaps it's better to comment them. If a variable is needed as parameter of a function, but is not used later, just name it _
an overload issue => need to check what's going on, could be a mismatching signature (The error might be related to this error that pops in the console React does not recognize the 'textWrap' prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase 'textwrap' instead. If you accidentally passed it from a parent component, remove it from the DOM element.)
a typing issue (property not existing on an unknown type) => need to check i this is a typo, really a typing issue or just a function/parameter somewhere misses a signature

You can find the details about the errors here:
https://github.com/MetaCell/c-elegans-app/actions/runs/9101844468/job/25020025355?pr=5

aranega · 2024-05-15T19:52:49Z

applications/visualizer/frontend/src/components/Workspace.tsx

-            </ThemeProvider>
-        </>
-    )
+  const isLoading = LayoutComponent === undefined;


@Salam-Dalloul Ok, got it! Thanks for the clarification

aranega · 2024-05-15T19:54:44Z

applications/visualizer/frontend/src/components/Workspace.tsx

-            </ThemeProvider>
-        </>
-    )
+  const isLoading = LayoutComponent === undefined;


@Salam-Dalloul I can see that at the end the isLoading is staying. I tried to remove it, but there is an issue, something is not loading well after. I guess that if we cannot remove it, then we can remove the use of <Suspense> all the point was to not use the isLoading variable :)

…LE-23

Salam-Dalloul · 2024-05-16T21:55:53Z

Hey @Salam-Dalloul I was reading the details for the issue CELE/23, according to the tasks, i find the following points missing( I am not sure if there was any discussion to not do the below points)

Search and Add from neurons, the dropdown is not like figma yet its empty( also change the placeholder value for the input)

bin icon to remove neuron

to create groups that makes a modal with input text for the group name appearing

per neuron to add neuron to a group through the menu linked to it

Datasets -dropdown with the list of all categories (filter as well).

For the UI

Height should be 56 as in figma

WIdth is 56 in figma

Border radius is 6 in figma

Width is 300px in figma

Yellow margin should be 12. not 24

Scrollbar is not functional

Text truncation is not working

Space between the development stage 1 heading the the items below is incorrect

9)Height in figma is 52

Margin top is wrong

Hover state for dataset option is missing

12. In figma its more of a rectangle than a circle 13. Tooltip for the switches is missing

@vidhya-metacell about

Search and Add from neurons, the dropdown is not like figma yet its empty( also change the placeholder value for the input)

Datasets -dropdown with the list of all categories (filter as well).

both have a separated tasks

3- Border radius is 6 in figma: it's .5rem in figma which is already implemented

5- Yellow margin should be 12. not 24: it's 1.5rem in figma and what is implemented

6- Scrollbar is not functional and 7- Text truncation is not working: tried it and was working fine, could you please re test this again?

Screen.Recording.2024-05-16.at.10.22.31.PM.mov

vidhya-metacell

Hey @Salam-Dalloul
I was reading the details for the issue CELE/23, according to the tasks, i find the following points missing( I am not sure if there was any discussion to not do the below points)

Search and Add from neurons, the dropdown is not like figma yet its empty( also change the placeholder value for the input)
bin icon to remove neuron
- to create groups that makes a modal with input text for the group name appearing
- per neuron to add neuron to a group through the menu linked to it
Datasets -dropdown with the list of all categories (filter as well).

For the UI

Height should be 56 as in figma
WIdth is 56 in figma
Border radius is 6 in figma
Width is 300px in figma
Yellow margin should be 12. not 24
Scrollbar is not functional
Text truncation is not working
Space between the development stage 1 heading the the items below is incorrect

9)Height in figma is 52

Margin top is wrong
Hover state for dataset option is missing

In figma its more of a rectangle than a circle

Tooltip for the switches is missing

Hey @Salam-Dalloul , things are looking pretty good now, only a few tiny misses this time

Please check the border radius in the active state.
In figma the total space between the dataset description and search is 1.5, but in your case if you notice there is margin 1.5 but also a padding coming in the bottom
of the description box which inturn increases the whole space

There needs to be a gap of 0.5 between the Development stage 1 and ul, the gap exists between the list items which is good, but there is none between the heading and the list

the below is from the code, where youc an see no gap
Tooltip on the switch is behaving weird with only showing on the second switch

Screen.Recording.2024-05-17.at.5.44.06.PM.mov

Salam-Dalloul · 2024-05-20T09:55:43Z

Hey @Salam-Dalloul I was reading the details for the issue CELE/23, according to the tasks, i find the following points missing( I am not sure if there was any discussion to not do the below points)

Search and Add from neurons, the dropdown is not like figma yet its empty( also change the placeholder value for the input)

bin icon to remove neuron

to create groups that makes a modal with input text for the group name appearing

per neuron to add neuron to a group through the menu linked to it

Datasets -dropdown with the list of all categories (filter as well).

For the UI

Height should be 56 as in figma

WIdth is 56 in figma

Border radius is 6 in figma

Width is 300px in figma

Yellow margin should be 12. not 24

Scrollbar is not functional

Text truncation is not working

Space between the development stage 1 heading the the items below is incorrect

9)Height in figma is 52

Margin top is wrong

Hover state for dataset option is missing

12. In figma its more of a rectangle than a circle 13. Tooltip for the switches is missing Hey @Salam-Dalloul , things are looking pretty good now, only a few tiny misses this time

Please check the border radius in the active state.

In figma the total space between the dataset description and search is 1.5, but in your case if you notice there is margin 1.5 but also a padding coming in the bottom
of the description box which inturn increases the whole space

There needs to be a gap of 0.5 between the Development stage 1 and ul, the gap exists between the list items which is good, but there is none between the heading and the list

the below is from the code, where youc an see no gap

Tooltip on the switch is behaving weird with only showing on the second switch

Screen.Recording.2024-05-17.at.5.44.06.PM.mov

@vidhya-metacell fixed all of them and about the border radius @syamace-metacell have updated the design to be united in both cases, to be .5rem

vidhya-metacell

@Salam-Dalloul looks good now

Feature/cele 25 Header UI

Salam-Dalloul added 5 commits May 4, 2024 20:49

#23 Left static sidebar

ef69bbc

- static left sidebar with 3 buttons - first button to compress the sidebar and leave only the minimal part visible

#23 add headers

2daed3e

#23 add Datasets items

7573da0

#23 add Neurons items

06f659e

#23 add filters section + color picker

95aff1d

Salam-Dalloul requested a review from ddelpiano May 10, 2024 11:17

Salam-Dalloul marked this pull request as ready for review May 10, 2024 11:17

#cele-25 Header UI

2188afd

ddelpiano requested review from vidhya-metacell and aranega and removed request for ddelpiano May 13, 2024 14:35

aranega requested changes May 13, 2024

View reviewed changes

#CELE-25 Modal updates

e123fd6

vidhya-metacell requested changes May 14, 2024

View reviewed changes

#cele-25 Modal fields update

bea9861

Salam-Dalloul changed the base branch from feature/CELE-22 to develop May 15, 2024 14:45

Salam-Dalloul and others added 4 commits May 15, 2024 18:17

fix conflict

e042665

use suspense for loading state

ea51ae6

Modify GH action to build without frozen-lockfile

2d88d83

Add missing lock-file

99a150f

aranega requested changes May 15, 2024

View reviewed changes

Salam-Dalloul and others added 3 commits May 16, 2024 19:01

styles fixes

d0d15e4

Merge remote-tracking branch 'origin/feature/CELE-23' into feature/CE…

0d6220b

…LE-23

CELE-23 Fix compilation issues

1344c02

aranega approved these changes May 16, 2024

View reviewed changes

Salam-Dalloul added 2 commits May 17, 2024 01:24

styles fixes

c787b3f

add Suspense to Workspace rendering

d90eea5

Salam-Dalloul requested review from vidhya-metacell and aranega May 16, 2024 22:59

#cele-25 Remove cross icon

59b9cda

vidhya-metacell requested changes May 17, 2024

View reviewed changes

vidhya-metacell and others added 3 commits May 17, 2024 18:37

#cele-25 Fix merge conflicts

c4f8cf9

#cele-25 Header menu ui

8bce19e

styles fixes + fix switch issue

8549f1b

Salam-Dalloul requested a review from vidhya-metacell May 20, 2024 09:57

fix hover background color for bin icon

1c72a8b

vidhya-metacell approved these changes May 21, 2024

View reviewed changes

vidhya-metacell and others added 2 commits May 21, 2024 11:56

#cele-25 Compare workspace dialog

91cd354

Merge pull request #7 from MetaCell/feature/CELE-25

494c7b1

Feature/cele 25 Header UI

ddelpiano merged commit e8477d2 into develop May 22, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#23 Left static sidebar #5

#23 Left static sidebar #5

Salam-Dalloul commented May 4, 2024 •

edited

Loading

aranega left a comment •

edited

Loading

aranega May 13, 2024

aranega May 13, 2024

Salam-Dalloul May 15, 2024

aranega May 15, 2024

aranega May 15, 2024

Salam-Dalloul May 16, 2024 •

edited

Loading

vidhya-metacell left a comment

aranega left a comment •

edited

Loading

aranega May 15, 2024

aranega May 15, 2024

Salam-Dalloul commented May 16, 2024 •

edited

Loading

vidhya-metacell left a comment •

edited

Loading

Salam-Dalloul commented May 20, 2024

vidhya-metacell left a comment

#23 Left static sidebar #5

#23 Left static sidebar #5

Conversation

Salam-Dalloul commented May 4, 2024 • edited Loading

aranega left a comment • edited Loading

Choose a reason for hiding this comment

aranega May 13, 2024

Choose a reason for hiding this comment

aranega May 13, 2024

Choose a reason for hiding this comment

Salam-Dalloul May 15, 2024

Choose a reason for hiding this comment

aranega May 15, 2024

Choose a reason for hiding this comment

aranega May 15, 2024

Choose a reason for hiding this comment

Salam-Dalloul May 16, 2024 • edited Loading

Choose a reason for hiding this comment

vidhya-metacell left a comment

Choose a reason for hiding this comment

aranega left a comment • edited Loading

Choose a reason for hiding this comment

aranega May 15, 2024

Choose a reason for hiding this comment

aranega May 15, 2024

Choose a reason for hiding this comment

Salam-Dalloul commented May 16, 2024 • edited Loading

vidhya-metacell left a comment • edited Loading

Choose a reason for hiding this comment

Salam-Dalloul commented May 20, 2024

vidhya-metacell left a comment

Choose a reason for hiding this comment

Salam-Dalloul commented May 4, 2024 •

edited

Loading

aranega left a comment •

edited

Loading

Salam-Dalloul May 16, 2024 •

edited

Loading

aranega left a comment •

edited

Loading

Salam-Dalloul commented May 16, 2024 •

edited

Loading

vidhya-metacell left a comment •

edited

Loading