Learning profile recharts poc #2831

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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+);
+
+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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+
+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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

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


this ain't right 🙈

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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+  }, 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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

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


style: maybe signature destructuring

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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

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


totalDataset, ça correspond à quoi?

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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+  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

packages/@coorpacademy-components/src/util/check-is-mobile.js

@@ -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

.../@coorpacademy-components/src/molecule/learning-profile-radar-chart/test/fixtures/hexagon.js

+      '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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+
+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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+// 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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

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


maybe const black, white, et main color?

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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+
+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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+  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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+  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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+        // 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

packages/@coorpacademy-components/src/molecule/learning-profile-radar-chart/index.js

+  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

...components/src/molecule/learning-profile-radar-chart/test/fixtures/hexagon-with-2-dataset.js

+      {
+        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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Learning profile recharts poc #2831

Learning profile recharts poc #2831

adamska27 commented Mar 13, 2024 •

edited

Loading

AdrieanKhisbe left a comment

AdrieanKhisbe Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

adamska27 Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

AdrieanKhisbe Mar 27, 2024

codecov bot commented Mar 28, 2024 •

edited

Loading

AdrieanKhisbe Mar 28, 2024

adamska27 Apr 2, 2024

AdrieanKhisbe Mar 28, 2024

AdrieanKhisbe Mar 28, 2024

AdrieanKhisbe Mar 28, 2024

AdrieanKhisbe Mar 28, 2024

AdrieanKhisbe Mar 28, 2024

AdrieanKhisbe Mar 28, 2024

AdrieanKhisbe Mar 28, 2024

adamska27 Apr 2, 2024

AdrieanKhisbe Mar 28, 2024

		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 = {

Learning profile recharts poc #2831

Are you sure you want to change the base?

Learning profile recharts poc #2831

Conversation

adamska27 commented Mar 13, 2024 • edited Loading

AdrieanKhisbe 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

codecov bot commented Mar 28, 2024 • edited Loading

Codecov Report

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

adamska27 commented Mar 13, 2024 •

edited

Loading

codecov bot commented Mar 28, 2024 •

edited

Loading