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

Fix: issue 3438 #6748

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

Anemaygi
Copy link

Caution

The Volto Team has suspended its review of new pull requests from first-time contributors until the release of Plone 7, which is preliminarily scheduled for the second quarter of 2026.
Read details.



If your pull request closes an open issue, include the exact text below, immediately followed by the issue number. When your pull request gets merged, then that issue will close automatically.

Closes #

Copy link

boring-cyborg bot commented Feb 12, 2025

[!CAUTION] > The Volto Team has suspended its review of new pull requests from first-time contributors until the release of Plone 7, which is preliminarily scheduled for the second quarter of 2026. > Read details.
Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, read Plone's Code of Conduct, Contributing to Plone, First-time contributors, and Contributing to Volto, as this will greatly help the review process.

Welcome to the Plone community! 🎉

Copy link

netlify bot commented Feb 12, 2025

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 12fb7fc
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/67ad665df543150008a7a01f

stevepiercy added a commit that referenced this pull request Feb 12, 2025
- See broken example at #6748 (comment)

This PR does not need a changelog, as it's a follow up to #6613
@Anemaygi
Copy link
Author

Context

I have added the logic for the different use cases.

In the test file for this component (packages/volto/src/components/theme/View/EventDatesInfo.test.jsx),
the expected structure is defined by the test snapshot (packages/volto/src/components/theme/View/snapshots/EventDatesInfo.test.jsx.snap)

Even though the texts are the same, they still have problems in the automated tests. It is because we should also have a <span className="start"> and <span className="end"> encapsulating the start/end dates/times.

To fix:

  • Translation problem (why does our translation defined in packages/volto/locales/pt_BR/LC_MESSAGES/volto.po not show up?)
  • Automated tests errors

Is the code clear enough? :) please let me know!

@erral erral self-requested a review February 13, 2025 06:54
@erral
Copy link
Member

erral commented Feb 13, 2025

To fix:

* [ ]   Translation problem (why does our translation defined in `packages/volto/locales/pt_BR/LC_MESSAGES/volto.po` not show up?)

You need to run make i18n to convert the translations in po files to json files, and then they will be picked up by Volto.

* [ ]  Automated tests errors

Regarding the automated test errors, some of them are just whitespace things, that can be solved updating the snapshot (delete the file and run the tests to regenerate them).

In the other cases, where different HTML is outputed, I would try to keep the new HTML the same as the previous one, if possible.

The working of the new When_ component is correct, this is how it should be i18nized, using variables and not hardcoding the prepositions.

So big +1 for me, but you need to check the HTML produced by the new component.

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.

2 participants