feat: create research route #307

NoamGaash · 2023-12-15T16:35:14Z

Description

add a research secret route with stacked chart
solve #261 and #262

screenshots

github-actions · 2023-12-15T16:37:04Z

Preview: https://s3.amazonaws.com/noam-gaash.co.il/7231292194/open-bus/eee6f1985a269447a7f883441fbf77f76a27a37f/index.html
Preview Storybook: https://s3.amazonaws.com/noam-gaash.co.il/7231292194/open-bus/eee6f1985a269447a7f883441fbf77f76a27a37f/storybook/index.html

ArkadiK94 · 2023-12-15T23:55:28Z

as you can see the table is overflow to next chart.
In addition, please, sort the data in every chart by name/amount so it will be easier to find what we look for, by name-> to find the operator, by amount --> to see the worst/better performance

ArkadiK94 · 2023-12-15T23:57:21Z

Could you make the inputs smaller as in other parts of the website?

ArkadiK94

Great Job ,Looks amazing 💯
I have some minor comments if you would like to change.

ArkadiK94 · 2023-12-15T23:28:37Z

src/pages/DataResearch.tsx

+  return (
+    <PageContainer>
+      <Widget>
+        <h1>מחקרים</h1>


You should use here useTranslation and not Hebrew

it's a temporary internal thing, I don't think it should ever be translated

Ok, I understand

ArkadiK94 · 2023-12-15T23:30:06Z

src/pages/DataResearch.tsx

+
+  return (
+    <Widget>
+      <h1>בעיות etl/gps/משהו גלובאלי אחר</h1>


Also here and in some more places in this file , pls use useTranslation

src/routes/index.tsx

ArkadiK94 · 2023-12-16T00:11:16Z

src/pages/DataResearch.tsx

+  )
+}
+
+function StackedResearchSection() {


You should move it above the DataResearch component

As I wrote in different comment you don't have to

ArkadiK94 · 2023-12-16T00:16:11Z

src/pages/DataResearch.tsx

+  )
+}
+
+function StackedResearchInputs({


You should move it above the StackedResearchSection component

Hmm, you don't have to, but when I read the file from above to down you use functions and then declare them. However, you use the component in different file so it is not give an errors, up to you.

Usually, I prefer putting helper function and small utilities at the bottom, so when you read it you'll see the important things first.
But it's just an opinion
https://softwareengineering.stackexchange.com/questions/196997/helper-methods-placement

ArkadiK94 · 2023-12-16T00:51:50Z

In addition, I think we should have an option to choose the list of operators/exclude some of them it will help in extracting data, in my opinion, but it is for another issue.

yardenmezi · 2023-12-16T08:55:14Z

src/pages/DataResearch.tsx

+        <Grid xs={6} item>
+          <DateSelector
+            time={endDate}
+            onChange={(data) => setEndDate(data)}


Suggestion: It is better not to allow to choose an end date earlier than the start date

you're right, but since it's an internal endpoint, it's less important here.
However, it is a great idea to open an issue to fix that on the front page

NoamGaash · 2023-12-16T09:58:12Z

Thanks for the review,

regarding sorting - that's out of scope for this PR, we can do it in the future.
z-index - fixed.
Smaller inputs & sorting - well, it's an internal endpoint for research purposes, so I don't want to spend time on making it visually appealing. if anyone wants to improve the design, it can be done on future pull requests.

ArkadiK94

Only, one suggestion, if you want of course. Make the tooltip in a height of the chart and scroll-able.
You also can leave it as it is :)
Great Work 👍

NoamGaash · 2023-12-16T10:42:23Z

thanks @ArkadiK94 !
that's a great suggestion, but I'm not sure it's possible using rechart built-in components. The tooltip can't be hovered, therefore I don't see how can it be scrolled

ShayAdler · 2023-12-16T14:23:10Z

Thanks @NoamGaash that's amazing!

ShayAdler · 2023-12-16T14:32:06Z

In addition, I think we should have an option to choose the list of operators/exclude some of them it will help in extracting data, in my opinion, but it is for another issue.

@ArkadiK94 - Agreed. I'll open an issue on that

NoamGaash added 2 commits December 15, 2023 17:45

feat: create research route

c7e42b5

feat: add research stacked graph

29911dd

This was linked to issues Dec 15, 2023

Create a secret route /data-research #261

Closed

Add a research stack bar with all the operators execution rate #262

Closed

fix: tooltip

ec97477

NoamGaash requested review from shootermv, yardenmezi, ShayAdler, ArkadiK94 and xoRmalka December 15, 2023 17:20

NoamGaash added 4 commits December 15, 2023 20:12

feat: add more charts and description

7a09664

feat: make headers and widgets more semantic

d4b1b19

feat: geek easter egg

4b0c1df

docs: geek easter egg

595acf8

ArkadiK94 requested changes Dec 16, 2023

View reviewed changes

yardenmezi reviewed Dec 16, 2023

View reviewed changes

NoamGaash added 2 commits December 16, 2023 12:12

fix: rechart tooltip z-index

b7554fd

fix: lazy load research page

611447e

NoamGaash requested a review from ArkadiK94 December 16, 2023 10:16

ArkadiK94 approved these changes Dec 16, 2023

View reviewed changes

NoamGaash merged commit 244b222 into main Dec 16, 2023
13 checks passed

NoamGaash deleted the feat/research-route branch December 16, 2023 10:51

github-actions bot mentioned this pull request Dec 18, 2023

chore(main): release 1.0.0 #326

Merged

github-actions bot mentioned this pull request Jun 14, 2024

chore(main): release 1.0.0 #432

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create research route #307

feat: create research route #307

NoamGaash commented Dec 15, 2023 •

edited

Loading

github-actions bot commented Dec 15, 2023 •

edited

Loading

ArkadiK94 commented Dec 15, 2023

ArkadiK94 commented Dec 15, 2023

ArkadiK94 left a comment

ArkadiK94 Dec 15, 2023

NoamGaash Dec 16, 2023

ArkadiK94 Dec 16, 2023

ArkadiK94 Dec 15, 2023

ArkadiK94 Dec 16, 2023

NoamGaash Dec 16, 2023

ArkadiK94 Dec 16, 2023

ArkadiK94 Dec 16, 2023

NoamGaash Dec 16, 2023

ArkadiK94 Dec 16, 2023

NoamGaash Dec 16, 2023

ArkadiK94 Dec 16, 2023

ArkadiK94 commented Dec 16, 2023 •

edited

Loading

yardenmezi Dec 16, 2023

NoamGaash Dec 16, 2023

NoamGaash commented Dec 16, 2023 •

edited

Loading

ArkadiK94 left a comment

NoamGaash commented Dec 16, 2023 •

edited

Loading

ShayAdler commented Dec 16, 2023

ShayAdler commented Dec 16, 2023

feat: create research route #307

feat: create research route #307

Conversation

NoamGaash commented Dec 15, 2023 • edited Loading

Description

screenshots

github-actions bot commented Dec 15, 2023 • edited Loading

ArkadiK94 commented Dec 15, 2023

ArkadiK94 commented Dec 15, 2023

ArkadiK94 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

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

Choose a reason for hiding this comment

ArkadiK94 commented Dec 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NoamGaash commented Dec 16, 2023 • edited Loading

ArkadiK94 left a comment

Choose a reason for hiding this comment

NoamGaash commented Dec 16, 2023 • edited Loading

ShayAdler commented Dec 16, 2023

ShayAdler commented Dec 16, 2023

NoamGaash commented Dec 15, 2023 •

edited

Loading

github-actions bot commented Dec 15, 2023 •

edited

Loading

ArkadiK94 commented Dec 16, 2023 •

edited

Loading

NoamGaash commented Dec 16, 2023 •

edited

Loading

NoamGaash commented Dec 16, 2023 •

edited

Loading