Skip to content

Reusable HURUMap Components #1036

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

Merged
merged 26 commits into from
Mar 7, 2025
Merged

Conversation

kelvinkipruto
Copy link
Contributor

@kelvinkipruto kelvinkipruto commented Jan 28, 2025

Description

This PR makes HURUMap components reusable across multiple apps. This is done so as to reuse them for PesaYetu.
PS: This PR however does not move components that import assets/svg.

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

@kelvinkipruto kelvinkipruto self-assigned this Jan 29, 2025
@koechkevin
Copy link
Contributor

@kelvinkipruto We could break this PR to smaller PRs. I think it is really important in migrating PesaYetu.

@koechkevin koechkevin marked this pull request as ready for review February 24, 2025 08:55
@koechkevin koechkevin requested a review from a team February 24, 2025 11:49
@kilemensi
Copy link
Member

@CodeForAfrica/tech

@kilemensi
Copy link
Member

Can we get this reviewed & merged today @CodeForAfrica/tech ?

Copy link
Contributor

@m453h m453h left a comment

Choose a reason for hiding this comment

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

🚀
@kelvinkipruto , I have one question regarding the migration.... if a reusable HURUmap component is importing something from @commons-ui/next does that necessarily mean that it should be in @hurumap-next ?

@kilemensi
Copy link
Member

Besides the question from @m453h, is there anything holding this PR back @kelvinkipruto ?

@kelvinkipruto
Copy link
Contributor Author

@kilemensi Nothing else holding it back.

@m453h
Copy link
Contributor

m453h commented Feb 28, 2025

@kilemensi Nothing else holding it back.

@kelvinkipruto to add a bit more context on this....I had seen there are a couple of components in hurumap-core importing from @commons-ui/next as listed below

./src/Card/ActionArea.js
./src/Card/Card.js
./src/RichHeader/RichHeader.js
./src/DataVisualisationGuide/index.js
./src/HowItWorks/index.js
./src/Menu/index.js
./src/LineClampedRichTypography/LineClampedRichTypography.js
./src/Header/Header.js

In my mind I had thought they should automatically be in hurumap-next instead of hurumap-core

@kilemensi
Copy link
Member

I'm probably missing something but these are the components in @hurumap-core package @m453h :

drwxr-xr-x@    - clemence 30 Jan 09:17  Action
drwxr-xr-x@    - clemence 26 Feb 18:24  ChartTooltip
drwxr-xr-x@    - clemence 30 Jan 09:17  Download
.rw-r--r--@  478 clemence  7 Aug  2024  index.js
drwxr-xr-x@    - clemence 27 Feb 14:49  IndicatorTitle
drwxr-xr-x@    - clemence 30 Jan 09:17  Location
drwxr-xr-x@    - clemence 30 Jan 09:17  LocationHighlight
drwxr-xr-x@    - clemence 28 Oct  2024  LocationTag
drwxr-xr-x@    - clemence 27 Feb 14:49  Scope
drwxr-xr-x@    - clemence 27 Feb 14:49  Share

Card, RichHeader, etc., are not in @hurumap-core. Are we talking about the same package?

@kilemensi
Copy link
Member

... or are those the new components @kelvinkipruto is trying to move into @hurumap-core @m453h ?

@kelvinkipruto
Copy link
Contributor Author

@m453h @hurumap/next should have dependencies that rely directly on nextjs. They can then be reused across any package.

@kilemensi
Copy link
Member

kilemensi commented Feb 28, 2025

If it's missing, lets update the packages' respective READMEs @kelvinkipruto / @m453h :

  1. @commons-ui/core depends on react and @mui/material
  2. @commons-ui/next depends on @commons-ui/core + next
  3. @hurumap/core depends on @commons-ui/core + basic HURUmap components (e.g. charts using vega)
  4. @hurumap/next depends on @commons-ui/next + @hurumap/core + HURUmap components that require next

@m453h
Copy link
Contributor

m453h commented Feb 28, 2025

I'm probably missing something but these are the components in @hurumap-core package @m453h :

drwxr-xr-x@    - clemence 30 Jan 09:17  Action
drwxr-xr-x@    - clemence 26 Feb 18:24  ChartTooltip
drwxr-xr-x@    - clemence 30 Jan 09:17  Download
.rw-r--r--@  478 clemence  7 Aug  2024  index.js
drwxr-xr-x@    - clemence 27 Feb 14:49  IndicatorTitle
drwxr-xr-x@    - clemence 30 Jan 09:17  Location
drwxr-xr-x@    - clemence 30 Jan 09:17  LocationHighlight
drwxr-xr-x@    - clemence 28 Oct  2024  LocationTag
drwxr-xr-x@    - clemence 27 Feb 14:49  Scope
drwxr-xr-x@    - clemence 27 Feb 14:49  Share

Card, RichHeader, etc., are not in @hurumap-core. Are we talking about the same package?

these

Hold on

... or are those the new components @kelvinkipruto is trying to move into @hurumap-core @m453h ?

@kilemensi I was referring to the new components that are added/moved to @hurumap-core in this PR I believe the list you shared is on the main branch. This is the full list

❯ ls -lh
drwxr-xr-x   6 michael  staff   192B Feb 28 16:08 AboutTeam
drwxr-xr-x   6 michael  staff   192B Aug 19  2024 Action
drwxr-xr-x   9 michael  staff   288B Feb 28 16:08 Card
drwxr-xr-x   6 michael  staff   192B Feb 28 16:08 Carousel
drwxr-xr-x   7 michael  staff   224B Aug 19  2024 ChartTooltip
drwxr-xr-x   3 michael  staff    96B Feb 28 16:08 DataVisualisationGuide
drwxr-xr-x   7 michael  staff   224B Aug 19  2024 Download
drwxr-xr-x   6 michael  staff   192B Feb 28 16:08 Header
drwxr-xr-x   4 michael  staff   128B Feb 28 16:08 HowItWorks
drwxr-xr-x   6 michael  staff   192B Jan 15 00:12 IndicatorTitle
drwxr-xr-x   4 michael  staff   128B Feb 28 16:08 LineClampedRichTypography
drwxr-xr-x   4 michael  staff   128B Feb 28 16:08 Loading
drwxr-xr-x   6 michael  staff   192B Nov 22 12:03 Location
drwxr-xr-x   6 michael  staff   192B Aug 19  2024 LocationHighlight
drwxr-xr-x   6 michael  staff   192B Nov 22 12:03 LocationTag
drwxr-xr-x   4 michael  staff   128B Feb 28 16:08 Menu
drwxr-xr-x   6 michael  staff   192B Feb 28 16:08 PageHero
drwxr-xr-x   6 michael  staff   192B Feb 28 16:08 RichHeader
drwxr-xr-x  16 michael  staff   512B Feb 25 11:15 Scope
drwxr-xr-x   9 michael  staff   288B Dec 10 14:20 Share
drwxr-xr-x   6 michael  staff   192B Feb 28 16:08 Summary
-rw-r--r--   1 michael  staff   1.1K Feb 28 16:08 index.js

@kilemensi
Copy link
Member

Then I guess over to @kelvinkipruto ... I believe my previous message should clear where each component should be going.

Signed-off-by: Kipruto <[email protected]>
Copy link
Member

@kilemensi kilemensi left a comment

Choose a reason for hiding this comment

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

👍🏽


Looks like you still need to review what goes into which package.

@kelvinkipruto kelvinkipruto requested a review from kilemensi March 5, 2025 08:55
Copy link
Member

@kilemensi kilemensi left a comment

Choose a reason for hiding this comment

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

Haven't run it locally but code LGTM 🚀


Create an issue to move all payload related code in the current @hurumap/next package into a new @hurumap/payload package (similar to @commons-ui/payload)

@kelvinkipruto
Copy link
Contributor Author

Create an issue to move all payload related code in the current @hurumap/next package into a new @hurumap/payload package (similar to @commons-ui/payload)

Tracked in #1067

@kelvinkipruto kelvinkipruto added this pull request to the merge queue Mar 7, 2025
Merged via the queue into main with commit b6865bb Mar 7, 2025
4 checks passed
@kelvinkipruto kelvinkipruto deleted the chore/reusable-hurumap-components branch March 7, 2025 07:56
@github-project-automation github-project-automation bot moved this from In Progress to ✅ Done in COMMONS Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants