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

feature(website): merge existing webpages into /legal page #738

Merged
merged 14 commits into from
Feb 8, 2024

Conversation

DarkMenacer
Copy link
Contributor

No description provided.

Copy link

vercel bot commented Feb 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
public ❌ Failed (Inspect) Feb 8, 2024 2:40pm

Copy link

github-actions bot commented Feb 2, 2024

Visit the preview URL for this PR (updated for commit 3dab1b8):

https://si-admin-staging--pr738-pranav-merge-into-le-ktavgx9q.web.app

(expires Tue, 13 Feb 2024 22:25:30 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: b7b0969384059dce6ea8fad1ee1d1737e54e6676

@ssandino
Copy link
Member

ssandino commented Feb 5, 2024

Nice job @DarkMenacer – I made a commit with amendments to the DE translations, as well minor changes to the EN. I see some translations are not yet linked … but this you know of course.

3 things that I noticed (see image with numbering for context)

  1. I assume the title of the navigation you'll move to the shared file website-legal-json, I changed the EN title and added the DE as comment
  2. This title is not needed
  3. One icon in the list is smaller (let's see how it reacts with the actual translations, they should of course all be the same size)

I think we are almost there!

Screenshot 2024-02-05 at 17 47 22

@DarkMenacer
Copy link
Contributor Author

  1. Oops, my bad, I don't know how that got neglected while moving to translations
  2. Okay, I thought so, I'll remove those titles
  3. Once the actual title comes in, the icon regularises in size, so shouldn't be an issue hopefully but still I'll check the responsiveness again

@ssandino
Copy link
Member

ssandino commented Feb 6, 2024

@DarkMenacer 1. Good, 2. Good, 3. Good – thanks for these last changes and for switching from draft to normal PR for review? The PR will also be used to finish #712.

@DarkMenacer DarkMenacer marked this pull request as ready for review February 6, 2024 19:10
@DarkMenacer
Copy link
Contributor Author

Done
Screenshot 2024-02-07 at 00 40 50

@ssandino
Copy link
Member

ssandino commented Feb 6, 2024

  1. Once the actual title comes in, the icon regularises in size, so shouldn't be an issue hopefully but still I'll check the responsiveness again

I still had the issue with the slightly smaller icon:
Screenshot 2024-02-06 at 22 36 46

Found a easy work around with shorten the translation (so all good):
Screenshot 2024-02-06 at 22 36 37

Copy link
Member

@ssandino ssandino left a comment

Choose a reason for hiding this comment

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

Looks good to me. Once this PR is merged, we can proceed to delete the outdated legal pages (those not organized under the 'legal' path) and set up some redirects to avoid 404 errors.

@ssandino ssandino mentioned this pull request Feb 6, 2024
9 tasks
@mkue
Copy link
Contributor

mkue commented Feb 7, 2024

Very nice! Looks good to me :)

@ssandino ssandino merged commit d15a0bf into main Feb 8, 2024
17 of 19 checks passed
@ssandino ssandino deleted the pranav/merge-into-legal branch February 8, 2024 15:17
@ssandino ssandino restored the pranav/merge-into-legal branch February 8, 2024 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Website Feature]: Merge terms of use, terms and conditions, privacy policy into one page, /legal
3 participants