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.

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

Unify table on the home page #1065

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Unify table on the home page #1065

wants to merge 15 commits into from

Conversation

EshaanAgg
Copy link
Contributor

@EshaanAgg EshaanAgg commented Jul 25, 2024

I have added the basic functionality related to merging the UI of the two tables in the Overview component. I would highly appreciate it if @PhilippWendler, sir you could check if the table is performing as expected in terms of allowing the users to:

  • Select columns
  • Filter data
  • Resize columns
  • Sticky headers

After the same is completed, I would work on improving the UI as per the suggestions, and then would update the test suite as well as the build.

Fixes: #506

@EshaanAgg EshaanAgg marked this pull request as draft July 25, 2024 13:26
@PhilippWendler
Copy link
Member

Great! I think the main functionality is all there and behaves as expected.

If one deselects a complete group of column (disabling one row of columns in the "Select Columns" view), I think it should be hidden completely from the summary view. Right now the cells in the header rows remain.

@EshaanAgg
Copy link
Contributor Author

I have added the handling for the same as well! If a runset's all columns are hidden, now it does not render the column related to the same, and also shows a message at the end of the table (sort of a warning). Do you think we should keep the warning?

@EshaanAgg
Copy link
Contributor Author

PS. I just noticed that all works well and good right now if we hide the first runset completely, but some of the data is not displayed properly when the second column is completely hidden. Can you please point me to the appropriate function responsible for calculating these tableHeaders and colspans, so that I can make use of the same again to calculate the same correctly whenever the hidden runsets changes?

@PhilippWendler
Copy link
Member

I think some visual indicator of hidden columns and run sets is good to have, yes. Maybe not like a warning below but something in the table? I think in the main table the column borders remain visible when a column is hidden, so if you hide more columns you get thicker borders. Or some icon or so? Not sure, but we can iterate on this.

some of the data is not displayed properly when the second column is completely hidden

Hm, didn't see this right now.

Can you please point me to the appropriate function responsible for calculating these tableHeaders and colspans, so that I can make use of the same again to calculate the same correctly whenever the hidden runsets changes?

Do you mean the colspan in what is currently the benchmark setup table? Everything in that table is produced here, and colspan is called "width" there. You can see the result of this function in the JSON code that is written by table-generator into the HTML template.

@PhilippWendler
Copy link
Member

Maybe instead of removing columns, just set there width to 0 or make them otherwise invisible? Then you would not have to worry about colspan.

@EshaanAgg
Copy link
Contributor Author

Got it, sir! Having an info icon on the first row with a tooltip might be a good way to communicate the same. I have added that in the latest push. You can let me know how that looks and feels. Also the idea of setting the width to 0 worked like a charm, and avoided me having to go into all of the python code regarding the calculations involving the spans! Thanks.

@EshaanAgg EshaanAgg marked this pull request as ready for review July 28, 2024 06:54
@EshaanAgg EshaanAgg requested a review from PhilippWendler July 28, 2024 06:54
Copy link
Member

@PhilippWendler PhilippWendler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the info icon seems like a good idea, I like it.

Hiding the first runset breaks the content of the rows with limits etc. for me. Do you see this as well?

Except for this, I think functionality is fine! 👍 We can start thinking about polishing the UI design already. I have some suggestions for this, but if you think you have a better or alternative idea, I am completely open and would like to hear your ideas!

The style of "Click here to select columns" is different on the Summary and the Table tab, maybe needs some CSS adjustment?

The "Fixed row title" should probably be aligned to the left such that it is above the fixed column. But otherwise it is probably fine.

The heading "Benchmark Setup" does not describe it correctly anymore, maybe "Benchmark Setup & Statistics"? Or we get rid of it altogether, as most other tabs also do not have a header. We could also rename what is in the tab name, e.g., put "(Benchmark) Setup & Statistics" there?

The vertical alignment of the statistics rows and the row names in the left columns does not match for me.

On windows that are horizontally narrow and after scrolling to the right, there is no visual separator anymore between the fixed left-most column and the rest. Probably the border that is there should not scroll away.

I think I prefer the thicker borders between the run sets that were previously in the statistics table. But the borders between the column names could become thinner again. It is certainly not a bad idea if as much as possible looks similar to the table on the Table tab.

Not sure about the alternating gray backgrounds in the statistics parts of the table. They look pretty strong, are they the same gray as in the top part? Due to the fact that these statistics rows are semantically grouped into something like a tree (cf. the indentation of the row names), it might also be a good idea to keep these without alternating background colors, which in the draft makes it look more like a purely sequential list.

Maybe we can also improve the overall layout a little bit. Right now it looks a little bit like a table that has a header row in the middle (because there is this one bold row). I would think it could be good if we give some visual indication that either (a) the top half is altogether the header or that (b) there are two parts of the table, each with its own header.

For (a) maybe it is enough to remove the bold, and add some bottom border to the row with the column names?

For (b) maybe some more vertical space in the middle or so?

But if we manage to create (a) with a nice UI I guess I would prefer (a) over (b).

@@ -5,8 +5,6 @@
//
// SPDX-License-Identifier: Apache-2.0

"use strict";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is removing this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No sir. Will revert!

Copy link
Contributor Author

@EshaanAgg EshaanAgg Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently, it's an ESLint recommendation to remove the same, as the "use strict" directive is useless outside of ES modules and does not work for standalone scripts.
image

@EshaanAgg
Copy link
Contributor Author

I will be numbering the suggestions and the discussions so that it is easier to keep track of the same!

1

Yes, the info icon seems like a good idea, I like it.

Glad that you liked it. Have moved it's styling to the one global stylesheet.

2

Hiding the first runset breaks the content of the rows with limits etc. for me. Do you see this as well?

Yes sir. The problem stems from the fact that when all of the row data like the col spans and all are calculated, they are calculated to show the data in the first column. What I mean by that is, if a field is supposed to span two columns, then the return would be like [["Common Content", 2]] where the resulting array only has content for one of the first columns. Since now that is hidden, and we display only column no, no data is visible to us.

I was trying to fix the same but can't seem to find a simple way to do it. The best thing would be to recalculate the stats and everything, but that is a tedious task. I tried implementing a single recalculation of the col spans algorithm in JS but dropped the idea to the overwhelming number of edge cases: like what we have, we have 3 runsets, and one column spans 1-2 while the other spans the 3rd. Now to handle the cases involving the hiding of any one or two of the columns does not remain an easy algorithm.

Do you have any suggestions for the same?

3

The style of "Click here to select columns" is different on the Summary and the Table tab, maybe needs some CSS adjustment?

Fixed the styling in the latest push!

4

The "Fixed row title" should probably be aligned to the left such that it is above the fixed column. But otherwise it is probably fine.

Added styles to align to the left, and center the content of the checkbox and it's corresponding label in the latest push.

5

The heading "Benchmark Setup" does not describe it correctly anymore, maybe "Benchmark Setup & Statistics"? Or we get rid of it altogether, as most other tabs also do not have a header. We could also rename what is in the tab name, e.g., put "(Benchmark) Setup & Statistics" there?

Makes sense! Would remove the header from the the tab, and rename the tabs to Setup & Statistics. Putting Benchmark (Setup & Statistics) made the tab name too long, and did not look good IMO.

6

The vertical alignment of the statistics rows and the row names in the left columns does not match for me.

Can you share a screenshot of the same along with the dimensions of your screen? There might be some styling issues, as we are currently kind of relying on the CSS to reposition the rows correctly instead of ensuring the same by semantic structure.

7

On windows that are horizontally narrow and after scrolling to the right, there is no visual separator anymore between the fixed left-most column and the rest. Probably the border that is there should not scroll away.

Got it sir. Trying to fix the same.

8

I think I prefer the thicker borders between the run sets that were previously in the statistics table. But the borders between the column names could become thinner again. It is certainly not a bad idea if as much as possible looks similar to the table on the Table tab.

9

Not sure about the alternating gray backgrounds in the statistics parts of the table. They look pretty strong, are they the same gray as in the top part?

Yes, they are currently.

Due to the fact that these statistics rows are semantically grouped into something like a tree (cf. the indentation of the row names), it might also be a good idea to keep these without alternating background colors, which in the draft makes it look more like a purely sequential list.

I get your point, but removing the same led to the UI feeling a bit incoherent and broken. As I envision it, the whole point of these stripes is to make reading the table easier so that it is easier for the user to map the title to the actual row content. Since now the statistics are even a part of a "single" table, it makes more sense to keep them from a consistency POV.

These were my two cents, but if you feel that the same is not the best way to proceed with them, I can undo the alternating highlighting.

10

Maybe we can also improve the overall layout a little bit. Right now it looks a little bit like a table that has a header row in the middle (because there is this one bold row). I would think it would be good if we give some visual indication that either (a) the top half is the header altogether or that (b) there are two parts of the table, each with its own header.

For (a) maybe it is enough to remove the bold, and add some bottom border to the row with the column names?

For (b) maybe some more vertical space in the middle or so?

But if we manage to create (a) with a nice UI I guess I would prefer (a) over (b).

Got it sir. I have removed the bold text and instead added borders as a visual demarcator. Let me know how you feel above the same.

@PhilippWendler
Copy link
Member

2

Hiding the first runset breaks the content of the rows with limits etc. for me. Do you see this as well?

Yes sir. The problem stems from the fact that when all of the row data like the col spans and all are calculated, they are calculated to show the data in the first column. What I mean by that is, if a field is supposed to span two columns, then the return would be like [["Common Content", 2]] where the resulting array only has content for one of the first columns. Since now that is hidden, and we display only column no, no data is visible to us.

Hm, but shouldn't that be also solved by not hiding the columns, just setting their width to zero? If I use the browser dev tools to remove display: none from these cells, they appear nicely.

I was trying to fix the same but can't seem to find a simple way to do it. The best thing would be to recalculate the stats and everything, but that is a tedious task.

With "stats" you mean the colspan info? (Just making sure that we are not talking about the "statistics" part of the table, because logic for recomputing that already exists.)

I tried implementing a single recalculation of the col spans algorithm in JS but dropped the idea to the overwhelming number of edge cases: like what we have, we have 3 runsets, and one column spans 1-2 while the other spans the 3rd. Now to handle the cases involving the hiding of any one or two of the columns does not remain an easy algorithm.

The algorithm that we use so far is not that difficult: start with the full expanded list, then find sequences of duplicates, and count them. But maybe you don't need it.

For the rest I will have a look later.

@PhilippWendler
Copy link
Member

Thanks for your work, here are my responses for the rest of the items. Everything not responded to is fine.

I will be numbering the suggestions and the discussions so that it is easier to keep track of the same!

👍

3

The style of "Click here to select columns" is different on the Summary and the Table tab, maybe needs some CSS adjustment?

Fixed the styling in the latest push!

Good. I just noticed that there doesn't really seem to be a reason why we do not style it as a regular button, does it? Could you try out how it looks if you just let the browser render it as usual?

Maybe we can then even remove the weird "Click here to", if it is obvious that it is a button?

5

The heading "Benchmark Setup" does not describe it correctly anymore, maybe "Benchmark Setup & Statistics"? Or we get rid of it altogether, as most other tabs also do not have a header. We could also rename what is in the tab name, e.g., put "(Benchmark) Setup & Statistics" there?

Makes sense! Would remove the header from the the tab, and rename the tabs to Setup & Statistics. Putting Benchmark (Setup & Statistics) made the tab name too long, and did not look good IMO.

Looks nice. Maybe we can remove or reduce the vertical spacing above the table now?

6

The vertical alignment of the statistics rows and the row names in the left columns does not match for me.

Can you share a screenshot of the same along with the dimensions of your screen? There might be some styling issues, as we are currently kind of relying on the CSS to reposition the rows correctly instead of ensuring the same by semantic structure.

Sure:
grafik

The dark grey bottom border of the statistics cells looks unaligned and also the borders of the row with the column names.

8

I think I prefer the thicker borders between the run sets that were previously in the statistics table. But the borders between the column names could become thinner again. It is certainly not a bad idea if as much as possible looks similar to the table on the Table tab.

Did you want to write something here?

9

Due to the fact that these statistics rows are semantically grouped into something like a tree (cf. the indentation of the row names), it might also be a good idea to keep these without alternating background colors, which in the draft makes it look more like a purely sequential list.

I get your point, but removing the same led to the UI feeling a bit incoherent and broken. As I envision it, the whole point of these stripes is to make reading the table easier so that it is easier for the user to map the title to the actual row content. Since now the statistics are even a part of a "single" table, it makes more sense to keep them from a consistency POV.

These were my two cents, but if you feel that the same is not the best way to proceed with them, I can undo the alternating highlighting.

Still undecided. Note how the main table on the table tab does not have alternating background colors.

Btw: What happens if the number of rows in the top half of the table changes? In my test table the row with the column names always has a gray background, but there are some optional rows in the top half. How does it look if the number of rows changes? I guess we do want to always make the row with the column names highlighted in some way, and not let it have a white background?

Maybe we need a lighter gray for the alternating background and a somewhat darker gray for the background of the row with the column names?

10

Maybe we can also improve the overall layout a little bit. Right now it looks a little bit like a table that has a header row in the middle (because there is this one bold row). I would think it would be good if we give some visual indication that either (a) the top half is the header altogether or that (b) there are two parts of the table, each with its own header.
For (a) maybe it is enough to remove the bold, and add some bottom border to the row with the column names?
For (b) maybe some more vertical space in the middle or so?
But if we manage to create (a) with a nice UI I guess I would prefer (a) over (b).

Got it sir. I have removed the bold text and instead added borders as a visual demarcator. Let me know how you feel above the same.

Yes, I think this is better than before. We just have to fine-tune, for example right now the vertical border between the run sets is brighter than the vertical borders within columns in one run set, which looks weird and contradicts the semantics (columns of one run set belong together). Can you change this, please?

@EshaanAgg
Copy link
Contributor Author

Got it, sir! Will try to make all of the recommended changes by tomorrow!

@EshaanAgg
Copy link
Contributor Author

Sorry for the delay on my end! I have been trying to improve the styling and CSS per the requirements described. However, working with design is not my forte. I have been able to get most of the issues resolved, but the current blockers are:

  • The concept of having dark and light borders as in the React Table (In the React table, we create blank columns to show the "thicker" borders, which I want to avoid as far as possible, as it complicates the rendering logic and layout)
  • Having the first border of each row (the column with the title) has a thick border when we scroll on lower-width devices. This seems to be an issue with the Z Index, which I'm trying to debug.

Other than that, all the other points seem to be addressed (if I'm not missing any). Will re-iterate through the checklist soon after these fixes.

@EshaanAgg
Copy link
Contributor Author

EshaanAgg commented Aug 5, 2024

I have polished the UI a bit more and made the following changes:

  • Used the browser's styling for the buttons natively.
  • Reduced the margins around the table.
  • Added the concept of hard borders to separate runsets and the first column (headers).
  • Removed the alternating stripes from the lower part of the table.
  • Ensured that the header row of the bottom table always had a grey background color. I chose to keep the same gray shade as used in the alternating stripes above as having three shades of grey (the one for stripes, one on hover, and the other one for column headers) turned out to be cluttered, and the UI looked incoherent. Please let me know if you feel that the same should be changed.
  • The leftmost border now sticks with the title column on low-width screens while scrolling.
  • Made all the borders (vertical) uniform.

The only challenge that seems to be left is aligning the table properly. Since we are rendering one new table for each runset in the larger main table, aligning the rows seems troublesome. I am trying to manually fix the height proportions to see if it works, but it doesn't look too promising. I would appreciate any pointers or ideas on how to solve the same.

@PhilippWendler
Copy link
Member

Thanks, this definitively looks better. I will post a screenshot as a reference:

grafik

The horizontal borders seem relatively strong and with the alternating background color I think we don't need that. Can you make them as small as they were before?

The horizontal borders are unaligned at the point between the first and second column:
grafik

How would it look if we use the darker gray from before for the vertical borders between run sets?

And can we make the borders between the column headers inside a run set less strong?

It is still the case that some of the borders of columns inside a run set are stronger than the borders between run sets. I guess it is a good design pattern that borders between closely related elements should not be stronger than borders between unrelated elements.

And in general, whenever we are not sure, we can just orient ourselves on the previous design of the tables and just change what is necessary to change due to the merge. But wherever it fits well, we can just keep the old design.

How is the horizontal spacing (column width) decided? On the example table (svcomp-simple-cbmc-cpachecker.table.html) the columns of the left runset are noticeably narrower than the right runset.

@PhilippWendler
Copy link
Member

I found some additional request in #506 (comment): In vertically small windows, the horizontal scroll bar should be at the bottom of the browser window, and not be scrolled out of view vertically. If I remove overflow-x: scroll from #benchmark_setup the behavior is already almost what we want, just that the Fixed row title: and the Generated by ... then also start to scroll horizontally. These should remain horizontally fixed, but I guess this is doable?

@ricffb
Copy link
Contributor

ricffb commented Aug 8, 2024

Some details I noticed and features I would use:

  • There is a horizontal scrollbar showing at the bottom even if there is nothing to scroll.
  • The hover color eliminates all vertical dividers (was also the case in the old version)
  • The rightmost mem usage is very tight with the edge of the window ( you could try if a little padding improves the sequeezed feeling)
  • It would be cool if the hover highlighting would also work for the statistics;
  • Next to showing Quantile plots: Clicking on the "status" cell (eg. correct true, incorrect) of one tool could set the filter to all solved tasks by the tool with said status (and similar for the other fields)

@dbeyer
Copy link
Member

dbeyer commented Aug 8, 2024

Awesome! Many thanks! Everything I wanted in #506 is fulfilled!

@dbeyer
Copy link
Member

dbeyer commented Aug 8, 2024

I found some additional request in #506 (comment): In vertically small windows, the horizontal scroll bar should be at the bottom of the browser window, and not be scrolled out of view vertically. If I remove overflow-x: scroll from #benchmark_setup the behavior is already almost what we want, just that the Fixed row title: and the Generated by ... then also start to scroll horizontally. These should remain horizontally fixed, but I guess this is doable?

This would be great indeed!

@PhilippWendler
Copy link
Member

  • Next to showing Quantile plots: Clicking on the "status" cell (eg. correct true, incorrect) of one tool could set the filter to all solved tasks by the tool with said status (and similar for the other fields)

I guess this makes sense, thanks for the suggestion. @EshaanAgg But we would want to implement it separately, not in this PR.

@EshaanAgg
Copy link
Contributor Author

The horizontal borders seem relatively strong and with the alternating background color I think we don't need that. Can you make them as small as they were before?

Done. Reverted them back to as usual.

The horizontal borders are unaligned at the point between the first and second column

Fixed the alignment issue as well.

How would it look if we use the darker gray from before for the vertical borders between run sets?
And can we make the borders between the column headers inside a run set less strong?

Using a darker shade of grey seemed to break the consistency of the design. The best (and the simplest) UI according to me was keeping the same shade of grey for all borders, and then using thicker borders to separate runsets, and thinner ones to separate the columns of a runset. I have updated the same in the latest push. Please let me know how you feel about it.

How is the horizontal spacing (column width) decided? On the example table (svcomp-simple-cbmc-cpachecker.table.html) the columns of the left runset are noticeably narrower than the right runset.

It is automatically calculated by React Table.

I found some additional request in #506 (comment): In vertically small windows, the horizontal scroll bar should be at the bottom of the browser window, and not be scrolled out of view vertically. If I remove overflow-x: scroll from #benchmark_setup the behavior is already almost what we want, just that the Fixed row title: and the Generated by ... then also start to scroll horizontally. These should remain horizontally fixed, but I guess this is doable?

Added the fix for this as well. Now we use the browser's scrollbars for horizontal scrolling, the Fixed Row Title remains fixed on horizontal scrolling, and the Benchexec footer always remains horizontally centered on all pages.

  • There is a horizontal scrollbar showing at the bottom even if there is nothing to scroll.

Fixed.

  • The hover color eliminates all vertical dividers (was also the case in the old version)

Have changed the same to a lighter tone so that the borders are still visible.

  • The rightmost mem usage is very tight with the edge of the window ( you could try if a little padding improves the sequeezed feeling)

Didn't understand this. Can you please provide more context, or perhaps a screenshot?

  • It would be cool if the hover highlighting would also work for the statistics;

Sure. @PhilippWendler how does this sound to you? Should I try to implement the same?

  • Next to showing Quantile plots: Clicking on the "status" cell (eg. correct true, incorrect) of one tool could set the filter to all solved tasks by the tool with said status (and similar for the other fields)

Sure. As mentioned by @PhilippWendler sir, will try to add it another PR once this is merged.

@PhilippWendler
Copy link
Member

The horizontal borders seem relatively strong and with the alternating background color I think we don't need that. Can you make them as small as they were before?

Done. Reverted them back to as usual.

I think they are still thicker than before?

How would it look if we use the darker gray from before for the vertical borders between run sets?
And can we make the borders between the column headers inside a run set less strong?

Using a darker shade of grey seemed to break the consistency of the design. The best (and the simplest) UI according to me was keeping the same shade of grey for all borders, and then using thicker borders to separate runsets, and thinner ones to separate the columns of a runset. I have updated the same in the latest push. Please let me know how you feel about it.

Ok. Maybe let's try with making all of them smaller? For example same width as before.

How is the horizontal spacing (column width) decided? On the example table (svcomp-simple-cbmc-cpachecker.table.html) the columns of the left runset are noticeably narrower than the right runset.

It is automatically calculated by React Table.

Ah, so it is caused by the contents of all the cells. Well, I guess this makes sense.

I found some additional request in #506 (comment): In vertically small windows, the horizontal scroll bar should be at the bottom of the browser window, and not be scrolled out of view vertically. If I remove overflow-x: scroll from #benchmark_setup the behavior is already almost what we want, just that the Fixed row title: and the Generated by ... then also start to scroll horizontally. These should remain horizontally fixed, but I guess this is doable?

Added the fix for this as well. Now we use the browser's scrollbars for horizontal scrolling, the Fixed Row Title remains fixed on horizontal scrolling, and the Benchexec footer always remains horizontally centered on all pages.

Thanks. I guess we could make Fixed Row Title even remain fixed if it is unchecked (like the footer).

  • The rightmost mem usage is very tight with the edge of the window ( you could try if a little padding improves the sequeezed feeling)

Didn't understand this. Can you please provide more context, or perhaps a screenshot?

I guess what is meant is that there is simply not much space between the numbers in the right most cells and the window border (for narrow windows). But it seems this was the same before, so let's keep it for now.

  • It would be cool if the hover highlighting would also work for the statistics;

Sure. @PhilippWendler how does this sound to you? Should I try to implement the same?

Sure, I actually do not remember why we did not have it. I guess it should look just like in the main table. Ideally we would also always use the same color for all hover effects (and define it in a CSS variable).

Copy link
Member

@PhilippWendler PhilippWendler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! Here are a few minor things I found while clicking through the current state.

There is now more whitespace to the left of the text in the "options" cells. I think this only wastes space. Please try to keep the changes to the style minimal, i.e., change only what is now required due to the restructuring but keep everything else the same.

The column headers ("status" etc.) are clickable, but this is only indicated by the cursor style. Previously we also had a hover effect, can we re-add this?

The left-most column seems to no longer be resizable. Can this feature be restored?

I created a table where a line break happened in the left-most column occurred, and this messed up the alignment:
grafik
Can you please have a look?

Should we maybe move the info icon for hidden run sets to the cell where the "Select Columns" button is? On the one hand it seems good to have it at the top, on the other hand it fits well next to "Select Columns".

The introduction of the footer on the plot tabs seems to have lead to double vertical scroll bars. Having the footer is maybe nice, but then the scroll bars need to be fixed.

className="infoTooltipContainer"
onMouseEnter={(e) => {
const tooltip = e.currentTarget.querySelector(".infoTooltip");
tooltip.style.visibility = "visible";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to have less CSS in the JavaScript code. I think usually we handle visibility of DOM elements via CSS classes, can we do that here as well?

column={column}
className="header-data clickable"
title="Show Quantile Plot of this column"
style={{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move CSS to the stylesheets (also below). Not only is it more manageable there, but it also reduces the size of our snapshot tests.

@EshaanAgg
Copy link
Contributor Author

Hey! I have completed most of these required changes and will push the code soon! The only bottleneck is the layout breaking when the content of one of the rows spans more than one line. I have been trying to dynamically match the heights of the corresponding rows in the main table and the child statistics tables, but the same is causing multiple rerenders in a second, to the extend it causes blocking of the UI and the browser hanging. Still trying to debug the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Two separate tables on summary page
4 participants