Learning profile recharts poc by adamska27 · Pull Request #2831 · CoorpAcademy/components

adamska27 · 2024-03-13T16:58:15Z

Detailed purpose of the PR

Result and observation

hexagon with customize colors:

pentagon with one skill on focus:

quadrilateral:

triangle:

wip > multiple dataset:

Breaking changes ?
If checked, what have you broken ?
Extra lib ?
- Recharts

Testing Strategy

Already covered by tests
Manual testing
Unit testing

diferant behavior on mobile

…ction

AdrieanKhisbe

Set of feedbacks

AdrieanKhisbe · 2024-03-27T14:03:05Z

+);
+
+const buildSvgGradient = styles =>
+  map.convert({cap: false})(({gradient}, index) => {


extrait un mapUncapped au top level

const mapUncapped = map.convert({cap: false});

AdrieanKhisbe · 2024-03-27T14:04:40Z

+
+const buildSvgGradient = styles =>
+  map.convert({cap: false})(({gradient}, index) => {
+    const {fill, stroke} = gradient;


[style] tu peux eventuelement le destructurer dans la signature

({gradient: {fill, stroke}}, index)=> (<svg>)

et utiliser la currification pour faire sauter le styles => KEEP(THIS, styles)

AdrieanKhisbe · 2024-03-27T14:06:25Z

+const right = {
+  offset: {x: 30, y: -10},
+  alignment: 'start',
+  marginRigth: 'auto'


this ain't right 🙈

AdrieanKhisbe · 2024-03-27T14:08:08Z

+  }, styles);
+
+// TICK_POSITIONS
+const top = {


style: si ça rentre je rendrai ce block plus compact:

/* Ticks ** */ const top = {offset: {x: -100, y: -65}, alignment: 'center', margin: 'auto'}; const bottom = {/*...*/}; const right = {/*...*/}; const left = {/*...*/};

AdrieanKhisbe · 2024-03-27T14:11:05Z

+            const {color, background} = percentage;
+            const {color: colorLabel} = label;


style: maybe signature destructuring

AdrieanKhisbe · 2024-03-27T14:20:26Z

+
+LearningProfileRadarChart.propTypes = {
+  data: PropTypes.objectOf(PropTypes.arrayOf(PropTypes.number)).isRequired,
+  totalDataset: PropTypes.number.isRequired,


totalDataset, ça correspond à quoi?

AdrieanKhisbe · 2024-03-27T14:20:50Z

+LearningProfileRadarChart.propTypes = {
+  data: PropTypes.objectOf(PropTypes.arrayOf(PropTypes.number)).isRequired,
+  totalDataset: PropTypes.number.isRequired,
+  onClick: PropTypes.func,


ptetre à renommer (précise que on click on parle)

AdrieanKhisbe · 2024-03-27T14:21:44Z

+  data: PropTypes.objectOf(PropTypes.arrayOf(PropTypes.number)).isRequired,
+  totalDataset: PropTypes.number.isRequired,
+  onClick: PropTypes.func,
+  styles: PropTypes.arrayOf(


ça serait cool s'il supporte une simple valeur, sans avoir à la wrapper dans une array

AdrieanKhisbe · 2024-03-27T14:24:14Z

@@ -0,0 +1,3 @@
+const mobileUserAgentRegex = /iphone|ipad|ipod|android|blackberry|windows phone/g;
+
+export const isMobile = () => mobileUserAgentRegex.test(navigator.userAgent.toLowerCase());


quality, test it!
(mobileUserAgentRegex to CONSTANT_CASE

Suggested change

export const isMobile = () => mobileUserAgentRegex.test(navigator.userAgent.toLowerCase());

export const isMobile = (userAgent = navigator.userAgent) => mobileUserAgentRegex.test(userAgent.toLowerCase());

AdrieanKhisbe · 2024-03-27T14:24:56Z

+      'Adaptability and resilience': [15.6],
+      'Digital culture': [43.8],
+      'Problem solving': [56.4],


Idéalement pas de tableau à une valeur en entrée

codecov · 2024-03-28T15:21:36Z

Codecov Report

Attention: Patch coverage is 50.00000% with 44 lines in your changes are missing coverage. Please review.

Project coverage is 93.88%. Comparing base (bb34234) to head (a9d5965).
Report is 10 commits behind head on master.

❗ Current head a9d5965 differs from pull request most recent head aa29ae7. Consider uploading reports for the commit aa29ae7 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2831      +/-   ##
==========================================
- Coverage   94.88%   93.88%   -1.00%     
==========================================
  Files         396      387       -9     
  Lines        6133     5857     -276     
  Branches      401      413      +12     
==========================================
- Hits         5819     5499     -320     
- Misses        249      292      +43     
- Partials       65       66       +1

Files	Coverage Δ
...src/molecule/learning-profile-radar-chart/types.ts	`100.00% <100.00%> (ø)`
...oorpacademy-components/src/util/check-is-mobile.js	`66.66% <66.66%> (ø)`
...rc/molecule/learning-profile-radar-chart/index.tsx	`46.91% <46.91%> (ø)`

... and 15 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f38adf1...aa29ae7. Read the comment docs.

AdrieanKhisbe · 2024-03-28T16:23:02Z

+
+const uncappedMap = map.convert({cap: false});
+
+// TICK_POSITIONS


Suggested change

// TICK_POSITIONS

/* TICK_POSITIONS */

Utilise plutot le block comment pour délimiter tes sections
(et NE CRIE PAS STEUPLE 😅 )

AdrieanKhisbe · 2024-03-28T16:23:26Z

+// TICK_POSITIONS
+const top = {offset: {x: -100, y: -65}, alignment: 'center', margin: 'auto'};
+const bottom = {offset: {x: -100, y: 10}, alignment: 'center', margin: 'auto'};
+const right = {offset: {x: 30, y: -10}, alignment: 'start', marginRigth: 'auto'};


Suggested change

const right = {offset: {x: 30, y: -10}, alignment: 'start', marginRigth: 'auto'};

const right = {offset: {x: 30, y: -10}, alignment: 'start', marginRight: 'auto'};

AdrieanKhisbe · 2024-03-28T16:24:11Z

+// CONSTANTS
+const DEFAULT_COLORS = {
+  gradient: {
+    fill: ['#0062ffff', '#0062ffff'],


maybe const black, white, et main color?

AdrieanKhisbe · 2024-03-28T16:24:54Z

+
+const CHART_CONFIGS_BY_SIDE_COUNT = pipe(keyBy('sideCount'), mapValues('name'))(CHART_CONFIGS);
+
+const CUSTOM_DOT_DEFAULT_PROPS = {


style: on peut ptetre omettre le CUSTOM du nom des constantes

AdrieanKhisbe · 2024-03-28T16:30:28Z

+  if (active && size(payload))
+    return (
+      <div className={style.tooltip}>
+        <p>{label}</p>
+        <p>{payload[0].value}%</p>
+      </div>
+    );
+
+  return null;


style

Suggested change

if (active && size(payload))

return (

<div className={style.tooltip}>

<p>{label}</p>

<p>{payload[0].value}%</p>

</div>

);

return null;

return active && size(payload) ? (

<div className={style.tooltip}>

<p>{label}</p>

<p>{payload[0].value}%</p>

</div>

) : null;

AdrieanKhisbe · 2024-03-28T16:30:36Z

+  const {value: activeDotValue, label: activeDotLabel} = activeDot;
+  const label = get('payload.subject', payload);
+  const value = get(`payload[${dataKey}]`, payload);
+  const isCurrentDotActive = value === activeDotValue && label === activeDotLabel;


maybe inline (au formatage pret)

Suggested change

const {value: activeDotValue, label: activeDotLabel} = activeDot;

const label = get('payload.subject', payload);

const value = get(`payload[${dataKey}]`, payload);

const isCurrentDotActive = value === activeDotValue && label === activeDotLabel;

const isCurrentDotActive = get(`payload[${dataKey}]`, payload) === activeDot.value && get('payload.subject', payload) === activeDot.label;

AdrieanKhisbe · 2024-03-28T16:32:09Z

+        // only on mobile
+        // to handle dot style on hover (convert to click)
+        // use with the tooltip component


style with block comment, après le activeDot={{ }}

AdrieanKhisbe · 2024-03-28T16:35:57Z

+  const isMobile_ = useMemo(() => getIsMobile(useragent), [useragent]);
+
+  const setIsMobile_ = useCallback(() => {
+    setIsMobile(isMobile_);
+  }, [isMobile_]);
+
+  useEffect(() => {
+    setIsMobile_();
+    window.addEventListener('resize', setIsMobile_);
+
+    return () => {
+      window.removeEventListener('resize', setIsMobile_);
+    };
+  }, [setIsMobile_]);


question: c'est un pattern courant? (il est déjà en place dans d'autre composants?)

non d'ailleurs c'est pas vraiment nécessaire,
je modifie ça

AdrieanKhisbe · 2024-03-28T16:39:07Z

+      {
+        gradient: {
+          fill: ['#0062ffff', '#8000ff85'],
+          stroke: ['#0062ffff', '#8000FF ']


attention à la deuxième valeur

adamska27 added 2 commits March 13, 2024 17:56

install recahrts

b3026c1

wip: try lib in order to match design

418ccc5

adamska27 added the pr: 🚧 Work In Progress label Mar 13, 2024

adamska27 self-assigned this Mar 13, 2024

adamska27 added 8 commits March 15, 2024 14:38

wip poc: handle on dot click + try match design

7d169d2

simplication with use of foreignObject

d246e70

diferant behavior on mobile

add different fixtures types

24fd674

rename fixtures

9474112

rename component + add a new gradient + refacto build custom tick fun…

6b13424

…ction

refacto

1c7a1c8

rename

98df7cf

add new component with range values to fix display

aa29ae7

AdrieanKhisbe requested changes Mar 27, 2024

View reviewed changes

AdrieanKhisbe reviewed Mar 28, 2024

View reviewed changes

adamska27 force-pushed the learning-profile-recharts-poc branch from a9d5965 to aa29ae7 Compare April 3, 2024 09:32

		const {color, background} = percentage;
		const {color: colorLabel} = label;

		@@ -0,0 +1,3 @@
		const mobileUserAgentRegex = /iphone\|ipad\|ipod\|android\|blackberry\|windows phone/g;

		export const isMobile = () => mobileUserAgentRegex.test(navigator.userAgent.toLowerCase());

	export const isMobile = () => mobileUserAgentRegex.test(navigator.userAgent.toLowerCase());
	export const isMobile = (userAgent = navigator.userAgent) => mobileUserAgentRegex.test(userAgent.toLowerCase());


		const uncappedMap = map.convert({cap: false});

		// TICK_POSITIONS

	const right = {offset: {x: 30, y: -10}, alignment: 'start', marginRigth: 'auto'};
	const right = {offset: {x: 30, y: -10}, alignment: 'start', marginRight: 'auto'};


		const CHART_CONFIGS_BY_SIDE_COUNT = pipe(keyBy('sideCount'), mapValues('name'))(CHART_CONFIGS);

		const CUSTOM_DOT_DEFAULT_PROPS = {

Uh oh!

Conversation

adamska27 commented Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AdrieanKhisbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Mar 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adamska27 commented Mar 13, 2024 •

edited

Loading

codecov Bot commented Mar 28, 2024 •

edited

Loading