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(i18n): update tests for french and spanish #878

Merged
merged 17 commits into from
Aug 7, 2023

Conversation

mamadoudicko
Copy link
Contributor

@mamadoudicko mamadoudicko commented Aug 7, 2023

Description

Original PR: #738

Just fixed tests here. Skipped 3. I'll fix them on next friday

@mamadoudicko mamadoudicko temporarily deployed to preview August 7, 2023 10:50 — with GitHub Actions Inactive
@vercel
Copy link

vercel bot commented Aug 7, 2023

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

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Aug 7, 2023 10:55am
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 7, 2023 10:55am

@mamadoudicko mamadoudicko changed the title feat: i8n - french - spanish fix(i18n): update tests for french and spanish Aug 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

LOGAF Level 3 - /home/runner/work/quivr/quivr/frontend/lib/config/LocaleConfig/resources.ts

  1. The resources object is hardcoded. Consider fetching it from a source of truth or making it a prop if it can change.
  2. The resources object is quite large. Consider breaking it down into smaller, more manageable objects.

LOGAF Level 3 - /home/runner/work/quivr/quivr/frontend/app/chat/components/ChatsList/hooks/useChatsList.ts

  1. Avoid using void with async functions. Instead, use try-catch block to handle errors.
  2. The useEffect hook is missing dependencies. Add getChats and setAllChats to the dependencies array.

Example:

useEffect(() => {
  const fetchAllChats = async () => {
    try {
      const response = await getChats();
      setAllChats(response.reverse());
    } catch (error) {
      console.error(error);
      publish({
        variant: "danger",
        text: t("errorFetching",{ ns : 'chat'})
      });
    }
  };
  fetchAllChats();
}, [getChats, setAllChats]);

LOGAF Level 3 - /home/runner/work/quivr/quivr/frontend/app/user/components/BrainConsumption.tsx

  1. The BrainConsumption component is directly manipulating the DOM to create a clip path for the fillingIcon. Consider using SVG elements and properties to create the clip path instead.
<svg>
  <clipPath id="clip">
    <rect x="0" y="0" width="100%" height={`${(1 - brainFilling) * 100}%`} />
  </clipPath>
  <GiBrain style={{ clipPath: 'url(#clip)' }} />
</svg>
  1. The BrainConsumption component is directly manipulating the DOM to set the size of the backgroundIcon and fillingIcon. Consider using CSS classes or inline styles instead.
<GiBrain className="w-full h-full fill-green-200 stroke-black stroke-1" />

🔧🔗🧠


Powered by Code Review GPT

@mamadoudicko
Copy link
Contributor Author

Well done @B0rrA!

@mamadoudicko mamadoudicko merged commit b0514d6 into main Aug 7, 2023
StanGirard pushed a commit that referenced this pull request Sep 12, 2023
* add libraries for traslation purposes

* Add button and service for language selection

* add spanish translation on login page

* add spanish translation on upload page

* Add spanish translations for explore page

* Add translations on user page

* Add translations for config page

* Add spanish translations on chat page

* add translations for brain page

* fix GUI and save on local storage

* fix (i18n) init and types

* fix (i18n): typos

* add translation on new brain modal

* add translations on metadata

* Add translations on home page

* fixes types

* fix(frontend-tests): use get by id instead of text

---------

Co-authored-by: Gustavo Maciel <gustavo_m13@outlook.com>
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.

3 participants