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

feat: new tutorial landing page version #134

Merged
merged 27 commits into from
Nov 18, 2024

Conversation

ricardoaerobr
Copy link
Contributor

What is the purpose of this pull request?

What problem is this solving?

How should this be manually tested?

Screenshots or example usage

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Requires change to documentation, which has been updated accordingly.

Copy link

netlify bot commented Nov 1, 2024

Deploy Preview for leafy-mooncake-7c2e5e ready!

Name Link
🔨 Latest commit c98f23b
🔍 Latest deploy log https://app.netlify.com/sites/leafy-mooncake-7c2e5e/deploys/6736643cad0fde0008ef749e
😎 Deploy Preview https://deploy-preview-134--leafy-mooncake-7c2e5e.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['release-no', 'release-auto', 'release-patch', 'release-minor', 'release-major']

src/messages/en.json Outdated Show resolved Hide resolved
src/messages/en.json Outdated Show resolved Hide resolved
src/messages/en.json Outdated Show resolved Hide resolved
src/messages/en.json Outdated Show resolved Hide resolved
src/messages/en.json Outdated Show resolved Hide resolved
src/messages/pt.json Outdated Show resolved Hide resolved
src/messages/pt.json Outdated Show resolved Hide resolved
src/messages/pt.json Outdated Show resolved Hide resolved
src/messages/pt.json Outdated Show resolved Hide resolved
src/messages/pt.json Outdated Show resolved Hide resolved
@PedroAntunesCosta
Copy link
Contributor

@julia-rabello I think many of this weird string changes are due to not updating an old branch from main. Many of these I recognize from the PR I merged with sentence case fixes and feedbacks from localization.

Copy link
Contributor

@PedroAntunesCosta PedroAntunesCosta left a comment

Choose a reason for hiding this comment

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

@ricardoaerobr in addition to the comments Júlia made above, I would like to point, as we talked earlier in our sync, that the links in the cards don't work. They point to a 404 tutorial. I consider that, given we already know the slugs we want each card to point to, a page with placeholders is not ready for code review.

Please add the slugs to each card, corresponding to the category slug found in public/navigation.json.

@PedroAntunesCosta
Copy link
Contributor

Also, please add a release label. I believe release-minor is appropriate.

@ricardoaerobr ricardoaerobr added the release-minor Minor version bump label Nov 6, 2024
Co-authored-by: Júlia Rabello <[email protected]>
ricardoaerobr and others added 18 commits November 6, 2024 10:10
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Pedro Antunes <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Co-authored-by: Júlia Rabello <[email protected]>
Copy link
Contributor

@julia-rabello julia-rabello left a comment

Choose a reason for hiding this comment

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

  • Card links are still wrong. They should not be tutorials/{categoryName}. They should be {docs/tutorial/{categoryName}
  • I noticed not all capitalization adjustments were rolled back / synced with main (e.g. Portal del desarrollador was changed to Portal del Desarrolador)

src/messages/pt.json Outdated Show resolved Hide resolved
src/messages/pt.json Outdated Show resolved Hide resolved
Copy link
Contributor

@PedroAntunesCosta PedroAntunesCosta left a comment

Choose a reason for hiding this comment

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

The card links are wrong.

https://deploy-preview-134--leafy-mooncake-7c2e5e.netlify.app/pt/docs/docs/tutorial/produtos-beta

Copy link
Contributor

@julia-rabello julia-rabello left a comment

Choose a reason for hiding this comment

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

Could we use category instead of cat (both in navigation.json and messages)? Looking at the result, it would be weird to have slugs like cat-apps, cat-sellers, cat-shipping 🐱

src/messages/pt.json Outdated Show resolved Hide resolved
Copy link
Contributor

@PedroAntunesCosta PedroAntunesCosta left a comment

Choose a reason for hiding this comment

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

It is not working, I am getting 500 in the category pages.
image

@julia-rabello
Copy link
Contributor

@ricardoaerobr sorry, I think you were correct before not using Tutorials between ' '. Now the deploy fails with this error:

image

@ricardoaerobr ricardoaerobr merged commit b94854d into main Nov 18, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-minor Minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants