Feature/test rechart #723

JeanMarcMilletScality · 2024-05-23T16:28:38Z

Component:

Description:

Design:

Breaking Changes:

[] Breaking Changes

Closes: #ISSUE_NUMBER

…h tooltip

…ry slider, and utils with calc functions

bert-e · 2024-05-23T16:28:41Z

Hello jeanmarcmilletscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2024-05-23T16:28:51Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

bert-e · 2024-05-23T16:40:05Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

bert-e · 2024-05-30T12:12:52Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

JBWatenbergScality · 2024-06-24T12:00:11Z

src/lib/components/globalhealthbar/CustomTooltip.tsx

+      // console.log('tooltip', tooltipRef.current);
+      // console.log('tooltipCoord', tooltipRef.current.getBoundingClientRect());
+      // left and top < 0 = tooltip is out of the screen
+      // right or bottom > window.innerWidth or window.innerheight = tooltip is out of the screen


Suggested change

// console.log('tooltip', tooltipRef.current);

// console.log('tooltipCoord', tooltipRef.current.getBoundingClientRect());

// left and top < 0 = tooltip is out of the screen

// right or bottom > window.innerWidth or window.innerheight = tooltip is out of the screen

// left and top < 0 = tooltip is out of the screen

// right or bottom > window.innerWidth or window.innerheight = tooltip is out of the screen

bert-e · 2024-06-24T12:00:16Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

JBWatenbergScality

I think we should unit test this component and ensure that the screen readers description of it is intelligible

JBWatenbergScality · 2024-06-24T12:05:39Z

src/lib/components/globalhealthbar/HistoryProvider.tsx

+type HistoryAlertContextType = {
+  selectedDate: number;
+  setSelectedDate: (date: number) => void;
+};
+export const HistoryAlertContext = createContext<
+  HistoryAlertContextType | undefined
+>(undefined);
+
+export const HistoryAlertProvider = ({ children }) => {
+  const [selectedDate, setSelectedDate] = useState<number>(Date.now());
+  return (
+    <HistoryAlertContext.Provider value={{ selectedDate, setSelectedDate }}>
+      {children}
+    </HistoryAlertContext.Provider>
+  );
+};
+
+export const useHistoryAlert = () => {
+  const context = useContext(HistoryAlertContext);
+  if (!context) {
+    return { selectedDate: null };
+  }
+  return context;
+};


This component is not specific to alerts, it is more a "Point in time" selector/provider that can be used more widely.

JBWatenbergScality · 2024-06-24T12:06:24Z

src/lib/components/globalhealthbar/HistorySlider.tsx

+
+const StyledRange = styled.input`
+  width: 600px;
+  padding: 0; /* nécessaire pour IE */


Please review those french comments

JBWatenbergScality · 2024-06-24T12:08:04Z

src/lib/components/globalhealthbar/HistorySlider.tsx

+  endDate: number;
+};
+
+export const HistoryAlertSlider = ({


Similarly I think this could be used in other context than Alerting. It is a "Point in time Slider/Selector", not sure how to name it yet

JBWatenbergScality · 2024-06-24T12:09:28Z

src/lib/components/globalhealthbar/HistorySlider.tsx

+  start,
+  end,
+  startDate,
+  endDate,


I'm not sure to understand why we have both Start and StartDate props, same for end and endDate

JBWatenbergScality · 2024-06-24T12:12:04Z

src/lib/components/globalhealthbar/utils.tsx

+    const width = ((selectedDate - startDate) / (endDate - startDate)) * 600;
+    const leftPosition = width - 132 / 2;
+    return `auto auto -4px ${leftPosition}px`;


From a reader perspective it looks like math black magic here, maybe we can introduce variables to explain what is

600

132

-4px ?

JBWatenbergScality · 2024-06-24T12:14:10Z

src/lib/components/globalhealthbar/utils.tsx

+  start: number,
+  end: number,
+  startDate: number,
+  endDate: number,


What the meaning of those props ? IMO the signature of the function is not very clear to understand what it does

JBWatenbergScality · 2024-06-24T12:16:22Z

stories/GlobalHealthBar/globalhealthbarRecharts.stories.tsx

+          DATE_FORMATER.format(history.selectedDate) +
+          'T' +
+          TIME_SECOND_FORMATER.format(history.selectedDate)


history.selectedDate.toISOString ?

JBWatenbergScality · 2024-06-24T12:19:15Z

src/lib/components/globalhealthbar/GlobalHealthBarRecharts.component.tsx

+            offset={20}
+            isAnimationActive={false}
+            cursor={false}
+            content={<CustomTooltip tooltipData={tooltipData}></CustomTooltip>}


nit: could be self-closed

JBWatenbergScality · 2024-06-24T12:20:19Z

src/lib/components/globalhealthbar/GlobalHealthBarRecharts.component.tsx

+          allowDataOverflow={true}
+          dataKey="start"
+          type="number"
+          domain={[new Date(start).getTime(), new Date(end).getTime()]}


Suggested change

domain={[new Date(start).getTime(), new Date(end).getTime()]}

domain={[startDate, endDate]}

JBWatenbergScality · 2024-06-24T12:20:43Z

src/lib/components/globalhealthbar/GlobalHealthBarRecharts.component.tsx

+          tickSize={8}
+          minTickGap={0}
+          interval={0}
+          ticks={getDataListOptions(startDate, endDate)}


Suggested change

ticks={getDataListOptions(startDate, endDate)}

ticks={getTicks(startDate, endDate)}

bert-e · 2024-06-24T12:24:32Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@JBWatenbergScality

JBWatenbergScality and others added 8 commits March 14, 2024 00:35

GlobalHealthMonitoring: POC new implementation using Recharts

999899c

first version functional component, standard and history feature, wit…

d6e23a1

…h tooltip

init stories and guideline for new globalhealthbar

160f810

add history bar to stories

a468573

add functions to render ticks, bars with round corners, tooltip

cc84bd9

split GlobalHealthBarRecharts code in multiple files : tooltip, histo…

38c55c0

…ry slider, and utils with calc functions

remove ref for tooltip of history and add fixed value for width

6c6dfad

change value in storie to test next to start

b684938

put if statements in useEffect

70018a3

JeanMarcMilletScality marked this pull request as ready for review May 30, 2024 12:12

JBWatenbergScality reviewed Jun 24, 2024

View reviewed changes

JBWatenbergScality requested changes Jun 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/test rechart #723

Feature/test rechart #723

JeanMarcMilletScality commented May 23, 2024

bert-e commented May 23, 2024

bert-e commented May 23, 2024

bert-e commented May 23, 2024

bert-e commented May 30, 2024

JBWatenbergScality Jun 24, 2024

bert-e commented Jun 24, 2024

JBWatenbergScality left a comment

JBWatenbergScality Jun 24, 2024

JBWatenbergScality Jun 24, 2024

JBWatenbergScality Jun 24, 2024

JBWatenbergScality Jun 24, 2024

JBWatenbergScality Jun 24, 2024

JBWatenbergScality Jun 24, 2024

JBWatenbergScality Jun 24, 2024

JBWatenbergScality Jun 24, 2024

JBWatenbergScality Jun 24, 2024

JBWatenbergScality Jun 24, 2024

bert-e commented Jun 24, 2024

	domain={[new Date(start).getTime(), new Date(end).getTime()]}
	domain={[startDate, endDate]}

	ticks={getDataListOptions(startDate, endDate)}
	ticks={getTicks(startDate, endDate)}

Feature/test rechart #723

Are you sure you want to change the base?

Feature/test rechart #723

Conversation

JeanMarcMilletScality commented May 23, 2024

bert-e commented May 23, 2024

Hello jeanmarcmilletscality,

bert-e commented May 23, 2024

Waiting for approval

bert-e commented May 23, 2024

Waiting for approval

bert-e commented May 30, 2024

Waiting for approval

Choose a reason for hiding this comment

bert-e commented Jun 24, 2024

Waiting for approval

JBWatenbergScality 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

bert-e commented Jun 24, 2024

Waiting for approval