Skip to content
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

[Website Feature]: Migrate evidence page #592

Closed
2 tasks done
DarkMenacer opened this issue Oct 22, 2023 · 5 comments · Fixed by #620
Closed
2 tasks done

[Website Feature]: Migrate evidence page #592

DarkMenacer opened this issue Oct 22, 2023 · 5 comments · Fixed by #620
Assignees
Labels
website Issues concerning Website

Comments

@DarkMenacer
Copy link
Contributor

Is there an existing request for this feature?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe.

We should migrate the existing webpage Evidence (https://socialincome.org/ch/en/evidence) to the new website setup. The URL should stay the same

Data for all languages present (English, Italian, German) should be transferred as well

Describe the solution/feature

  • Add new webpage (website/src/app/[lang]/[country]/(website)/evidence)
  • Add text content to EN, IT and DE translation files (locales in shared)

Describe alternatives you've considered

No response

Criteria for Completion

No response

Anything else?

Ignore header and footer

Code of Conduct

  • I've read the Code of Conduct and understand my responsibilities as a member of the Social Income community
@DarkMenacer DarkMenacer added feature website Issues concerning Website labels Oct 22, 2023
@DarkMenacer DarkMenacer self-assigned this Oct 22, 2023
@DarkMenacer
Copy link
Contributor Author

Screenshot 2023-11-08 at 22 38 50

As seen in the image, the existing website has a modal component.

I checked the ui folder for modal component but the code seems to be missing the modal component.
How should I code out the modal component?
Should I use daisyui modal component by first installing daisy ui (by npm i -D daisyui@latest)?

@mkue
Copy link
Contributor

mkue commented Nov 8, 2023

No, we don't use DaisyUI anymore. We switched to shadcn.
It's called Dialog there. Check out the section-6-card.tsx file in the (website)/(home) directory. There you can see how it is used.

Or the documentation here: https://ui.shadcn.com/docs/components/dialog

@DarkMenacer
Copy link
Contributor Author

Oh I see. I'm sorry for the confusion. I'll look into this as well.

@DarkMenacer
Copy link
Contributor Author

I am done with this page and added data only in English.
This page was huge!
There were 2 approaches I could have gone about

  1. Brute code all the data (in the Dialog component specially)
  2. Prioritise scalability but adopt a non-standard approach (at least based on how the current code is written)

I chose the second approach for following reasons

  1. This was the fastest I could have completed this page. If I were to have brute-forced the data, I would have had to cater to all different links separately and it would have involved tremendous amount of redundancy
  2. The current structure of website-evidence.json is structured, it is very easy for someone with no experience with writing code to understand how to modify the data without ever bothering to change any code. This was possible only because data is completely decoupled from the code.

But this choice of mine has resulted in some not-so-good things as well. I am sure that this simply because I lack experience. I've tried my best to resolve most of them but few of them persist. Here are the most important ones which deserve a mention

  1. Slight modification of i18n (type string to type any)
  2. The warning Each child in a list should have a unique "key" prop. Check the top-level render call using <DialogDescription>. See https://reactjs.org/link/warning-keys for more information at div (which I tried my best to resolve but couldn't, though I'll try my best to resolve it)
  3. Nested map functions. As each section had different number of cards and each card had different number of paragraphs (and each paragraph had different number of parts), I ended up using nested map functions. I have a hunch that this might not be the best practice.

I could go on and on about this page but to spare you from reading extremely long messages, I would just end this here.
Please let me know what you think of what I've done. In hindsight I should have asked this before. But to make up for this mistake, I'll just put it out there that I absolutely don't mind even if you feel I should completely re-work how I've ended up making the page.

@DarkMenacer
Copy link
Contributor Author

Code is available on branch pranav/migrate-evidence-bank-page

@DarkMenacer DarkMenacer linked a pull request Nov 13, 2023 that will close this issue
@mkue mkue closed this as completed in #620 Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
website Issues concerning Website
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants