Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Isara UI fixes #1064

Merged
merged 21 commits into from
Oct 19, 2023
Merged

Isara UI fixes #1064

merged 21 commits into from
Oct 19, 2023

Conversation

meguiraun
Copy link
Contributor

hi,

The samples view page was not working correcly for our Isara. I believe the main reason was the cell logic, so instead of "faking" cells for our case I created the new method getSampleTableSingleCell, a simplfied version of getSampleTable but with less iterations for non existing cells, the logic was started to be very obfuscated.

I could not test against FlexHCD... how do you test for it?

@jbflo
Copy link
Member

jbflo commented Sep 27, 2023 •
edited
Loading

Hi @meguiraun ,

Maybe we can take some time to work on this together, because this PR break a bit the Table view for FLEXHCD.
getSampleTableSingleCell is what you'll use for ISARA
so getSampleTable should remain the same right ?

@meguiraun
Copy link
Contributor Author

meguiraun commented Sep 27, 2023 •
edited
Loading

Hi @meguiraun ,

Maybe we can take some time to work on this together, because this PR break a bit the Table view for FLEXHCD. getSampleTableSingleCell is what you'll use for ISARA so getSampleTable should remain the same right ?

correct! I just fixed the getSampleTable so it stays as before

@meguiraun meguiraun changed the title Isara UI fixes WIP: Isara UI fixes Sep 28, 2023
@meguiraun
Copy link
Contributor Author

meguiraun commented Sep 28, 2023 •
edited
Loading

I noticed the sample filtering has stopped working for some change in this branch, setting as WIP while I fix it

edit: fixed!

@meguiraun meguiraun changed the title WIP: Isara UI fixes Isara UI fixes Sep 28, 2023
jbflo
jbflo requested changes Sep 29, 2023
View reviewed changes
ui/src/components/SampleGrid/SampleGridTable.css Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Show resolved Hide resolved
axelboc
axelboc reviewed Oct 2, 2023
View reviewed changes
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
ui/src/containers/SampleGridTableContainer.jsx Outdated Show resolved Hide resolved
@jbflo
Copy link
Member

jbflo commented Oct 2, 2023

Hi @meguiraun
Can I push some changes to this branch ?
so i don't need to create another PR

@meguiraun
Copy link
Contributor Author

thank you guys for your comments! I will not be able to adress them in the next couple of days :(

Yeah, of course, @jbflo push to this branch as much as you want ;)

@jbflo jbflo marked this pull request as draft October 2, 2023 12:33
@jbflo jbflo marked this pull request as ready for review October 3, 2023 15:41
@jbflo
Copy link
Member

jbflo commented Oct 3, 2023

That should be it,
the Files are a bit huge in therm of line of code but it is for handling both SC content with multiple Cell or not

image
image

@jbflo
Copy link
Member

jbflo commented Oct 17, 2023

Hey @meguiraun
Just to let you know i was done with the changes. we may probably review/merge it before it got Conflict.

@meguiraun
Copy link
Contributor Author

hej @jbflo!

I did not have much chance of testing it, neither showing to our scientist, but yeah, better merge now before is late. We can always tweak things later

@meguiraun
Copy link
Contributor Author

and thanks for all the job :)!

@meguiraun
Copy link
Contributor Author

I cannot approve this myself since I am the MR creator, but I fully approve it.

@jbflo
Copy link
Member

jbflo commented Oct 17, 2023

Hey @marcus-oscarsson
We are a bit done with this, for now , you can review/merge it once you got the time.

next step is to simplifying the SampleGridTableContainer and convert it to function component
for another PR.

@marcus-oscarsson
Copy link
Member

Ok, made a quick review I spotted a few things. Great initiative to make it a functional component !

meguiraun and others added 20 commits October 17, 2023 18:20
@meguiraun @jbflo
reset filter in samplew view mode change
f83d9a5
@meguiraun @jbflo
hacking our sample list view
9ba4d69
@meguiraun @jbflo
initisalize to false
45ceda1
@meguiraun @jbflo
fix isara view to show pucks above 16
7a048f3
@meguiraun @jbflo
Fixes for isara like view
72a3111
@meguiraun @jbflo
do not render if no sample to display
140b742
@meguiraun @jbflo
do not break the page, just alert
9853bc2
@meguiraun @jbflo
bit of linting cleanup
dc48606
@meguiraun @jbflo
no console
7683abc
@meguiraun @jbflo
undo changes in getSampleTable
6da4083
@meguiraun @jbflo
fixing filtering
9d0eb69
@jbflo
We should not log/print queue json this make request slow
e8ff057
@jbflo
FIx unique key warning
8a6c113
@jbflo
adjust css for sample list view
dd7bea2
@jbflo
Update sample list view
1417497 
Fix filter  by cell and puck logic ,
specify for Single Cell Table View
move function from child to Parent
Add logic to control all Samples Menu
@jbflo
LINT / Run Format
1755277
@jbflo
function returns JSX, named should be prefixed with render
5740c17
@jbflo
Rebase to Develop
7582599 
LInt And replace render function  by a js function
@jbflo
Small adjust for small screen
f0a84ae
@jbflo
Remove commented line
459ee48
@jbflo jbflo force-pushed the isara_ui_fixes branch 2 times, most recently from a3273e4 to db62be8 Compare October 17, 2023 17:05
@marcus-oscarsson
Copy link
Member

Hey guys, are you still working on this or can I do a final pass and merge ?

@meguiraun
Copy link
Contributor Author

although I created myself I guess is now @jbflo the main developer 😜... looks good to me for merging

@jbflo
Copy link
Member

jbflo commented Oct 19, 2023

@marcus-oscarsson , We done thanks

@marcus-oscarsson marcus-oscarsson merged commit e882148 into develop Oct 19, 2023
13 checks passed
@marcus-oscarsson marcus-oscarsson deleted the isara_ui_fixes branch October 19, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axelboc axelboc axelboc left review comments

@marcus-oscarsson marcus-oscarsson marcus-oscarsson left review comments

@jbflo jbflo jbflo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@meguiraun @jbflo @marcus-oscarsson @axelboc