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

Cb lang #24

Merged
merged 21 commits into from
Feb 28, 2024
Merged

Cb lang #24

merged 21 commits into from
Feb 28, 2024

Conversation

maehr
Copy link
Member

@maehr maehr commented Feb 16, 2024

Pull request

Proposed changes

Types of changes

  • New feature (non-breaking change which adds functionality).
  • Enhancement (non-breaking change which enhances functionality)
  • Bug Fix (non-breaking change which fixes an issue).
  • Breaking change (fix or feature that would cause existing functionality to change).

Checklist

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Summary by CodeRabbit

  • New Features

    • Enhanced multilingual support across the site, dynamically fetching translated text for various elements based on the site's language setting.
    • Improved user experience by updating error messages and download link messages in the video player component.
    • Updated button labels, modal titles, and descriptive text to be language-dependent for a localized user experience.
  • Bug Fixes

    • Fixed static error messages in the video player component to display dynamic, language-dependent messages for better user interaction.
  • Documentation

    • Updated documentation to include details about the new multilingual support and dynamic content updates.

Copy link

changeset-bot bot commented Feb 16, 2024

⚠️ No Changeset found

Latest commit: 273e05d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

coderabbitai bot commented Feb 16, 2024

Warning

Rate Limit Exceeded

@maehr has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 59 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between 060ec2d and 273e05d.

Walkthrough

The recent updates focus on enhancing the multilingual capabilities of the site. By dynamically updating text elements like button labels, search results, and error messages based on the site's language settings, these changes aim to provide a more localized and user-friendly experience. Additionally, modifications include updating UI elements to reflect accurate translations and improving consistency in displayed information.

Changes

Files Change Summary
_includes/item/... Updated text elements to fetch translations dynamically for a localized user experience.
_includes/js/lunr-js.html Introduced results_found variable for dynamic and localized search result counts.
_includes/nav-search-lunr.html Updated search button text to use translation lookups.
_layouts/search.html Enhanced localization for search options, modal labels, and advanced search instructions.
_layouts/timeline.html Updated variable assignments and labels for consistency and improved date range display format.

Related issues

  • Multilingual site option #9: The enhancements to multilingual support directly align with the request for a multilingual site option, addressing the need for dynamic translation capabilities and improved localization.

Poem

In the digital garden, where code does bloom,
A rabbit hopped, under the moon's soft gloom.
"Let words flow free, in every tongue,
So every soul, old and young,
May find their way, in light or gloom."
🌍🐰✨
With every hop, the site did change,
Becoming a world, vast and strange.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

@maehr maehr requested a review from koilebeit February 16, 2024 16:14
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d7cb583 and 2afcc81.
Files ignored due to path filters (2)
  • _config.yml is excluded by: !**/*.yml
  • _data/translations.yml is excluded by: !**/*.yml
Files selected for processing (44)
  • _includes/cb/credits.html (1 hunks)
  • _includes/collection-banner.html (1 hunks)
  • _includes/data-download-modal.html (1 hunks)
  • _includes/feature/audio.html (1 hunks)
  • _includes/feature/image.html (1 hunks)
  • _includes/feature/nav-menu.html (1 hunks)
  • _includes/feature/pdf.html (1 hunks)
  • _includes/feature/video.html (1 hunks)
  • _includes/footer.html (2 hunks)
  • _includes/head/head.html (1 hunks)
  • _includes/index/data-download.html (1 hunks)
  • _includes/index/description.html (1 hunks)
  • _includes/index/objects.html (1 hunks)
  • _includes/index/time.html (1 hunks)
  • _includes/item/3d-model-viewer.html (1 hunks)
  • _includes/item/audio-player.html (1 hunks)
  • _includes/item/breadcrumbs.html (1 hunks)
  • _includes/item/browse-buttons.html (1 hunks)
  • _includes/item/child/audio-player.html (1 hunks)
  • _includes/item/child/citation-box.html (1 hunks)
  • _includes/item/child/compound-item-download-buttons.html (1 hunks)
  • _includes/item/child/compound-item-modal-gallery.html (2 hunks)
  • _includes/item/child/download-buttons.html (1 hunks)
  • _includes/item/child/image-gallery.html (1 hunks)
  • _includes/item/child/rights-box.html (1 hunks)
  • _includes/item/child/video-player.html (1 hunks)
  • _includes/item/citation-box.html (1 hunks)
  • _includes/item/download-buttons.html (1 hunks)
  • _includes/item/image-gallery.html (1 hunks)
  • _includes/item/pdf-embed.html (1 hunks)
  • _includes/item/rights-box.html (1 hunks)
  • _includes/item/video-player.html (1 hunks)
  • _includes/js/browse-js.html (1 hunks)
  • _includes/js/map-js.html (1 hunks)
  • _includes/js/table-js.html (2 hunks)
  • _includes/nav-search-lunr.html (1 hunks)
  • _includes/scroll-to-top.html (1 hunks)
  • _layouts/browse.html (2 hunks)
  • _layouts/data.html (1 hunks)
  • _layouts/default.html (1 hunks)
  • _layouts/home-infographic.html (1 hunks)
  • _layouts/item/item-page-base.html (1 hunks)
  • _layouts/item/item-page-full-width.html (1 hunks)
  • _layouts/search.html (2 hunks)
Additional comments: 57
_includes/index/description.html (2)
  • 3-3: The dynamic fetching of the title based on the site's language setting is implemented correctly. It uses a fallback to "Description" if the translation is not available, which is a good practice for ensuring content availability in case of missing translations.
  • 5-5: The dynamic fetching of the "Learn More" button text is correctly implemented, including the use of &raquo; for the arrow symbol. This approach maintains consistency with the original static content while enabling multilingual support.
_includes/item/child/video-player.html (1)
  • 10-10: The implementation for dynamically fetching the fallback message for the video player based on the site's language setting is correct. It also provides a default message in English, which is a good practice for ensuring the message is always available.
_includes/item/audio-player.html (1)
  • 9-9: The dynamic fetching of the fallback message and download link text for the audio player is correctly implemented. This approach ensures that the audio player component is fully localized, enhancing the user experience for non-English speakers.
_includes/item/child/audio-player.html (1)
  • 9-9: The implementation for dynamically fetching the fallback message and download link text for the child audio player based on the site's language setting is correct. Including a default message ensures that the content is always available, which is a good practice.
_layouts/data.html (1)
  • 18-18: The dynamic fetching of the header cell content for the "Link" column in the data table is correctly implemented. This change ensures that the table header is localized, which is an important aspect of providing a fully multilingual user interface.
_includes/item/video-player.html (1)
  • 10-10: The implementation for dynamically fetching the error message and download link text for the video player based on the site's language setting is correct. This approach ensures that the video player component is fully localized, enhancing the user experience for non-English speakers.
_layouts/default.html (1)
  • 7-7: The dynamic fetching of the "Skip to main content" link text based on the site's language setting is correctly implemented. This is a crucial aspect of accessibility and localization, ensuring that all users, regardless of language, can navigate the site effectively.
_includes/item/child/image-gallery.html (1)
  • 9-9: The implementation for dynamically fetching the text displayed below the image in the child image gallery based on the site's language setting is correct. This change ensures that the gallery component is fully localized, enhancing the user experience for non-English speakers.
_includes/item/pdf-embed.html (1)
  • 10-10: The dynamic fetching of the fallback text for the PDF embed component based on the site's language setting is correctly implemented. Including a default message ensures that the content is always available, which is a good practice for enhancing user experience across different languages.
_includes/item/image-gallery.html (1)
  • 9-9: The implementation for dynamically fetching the text displayed below the image in the image gallery based on the site's language setting is correct. This change ensures that the gallery component is fully localized, enhancing the user experience for non-English speakers.
_includes/item/breadcrumbs.html (1)
  • 8-9: The dynamic fetching of the breadcrumb link texts based on the site's language setting is correctly implemented. This approach ensures that the breadcrumb navigation is fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_includes/item/citation-box.html (1)
  • 7-12: The implementation for dynamically fetching the text for the card headers and data terms in the citation box based on the site's language setting is correct. This change ensures that the citation information is fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_includes/item/rights-box.html (1)
  • 7-12: The dynamic fetching of the card header and labels in the rights box based on the site's language setting is correctly implemented. This approach ensures that the rights information is fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_includes/item/child/citation-box.html (1)
  • 7-12: The implementation for dynamically fetching the text for the card headers and data terms in the child item's citation box based on the site's language setting is correct. This change ensures that the citation information for child items is fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_includes/item/child/rights-box.html (1)
  • 7-12: The dynamic fetching of the card header and rights-related content in the child item's rights box based on the site's language setting is correctly implemented. This approach ensures that the rights information for child items is fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_layouts/home-infographic.html (1)
  • 11-20: The implementation for dynamically fetching the titles of included components based on the site's language setting in the home infographic layout is correct. This change ensures that the titles for "Sample Items," "Top Subjects," and "Locations" are fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_includes/scroll-to-top.html (1)
  • 19-19: The dynamic fetching of the title and aria-label attributes for the "Back to Top" button based on the site's language setting is correctly implemented. This is a crucial aspect of accessibility and localization, ensuring that all users, regardless of language, can understand the button's purpose.
_includes/nav-search-lunr.html (1)
  • 12-12: The implementation for dynamically fetching the visually-hidden text for the search button based on the site's language setting is correct. This change ensures that the search functionality is fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_includes/index/time.html (1)
  • 11-13: The dynamic fetching of the card title and button text in the time component based on the site's language setting is correctly implemented. This change ensures that the time span information and the "View Timeline" button are fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_includes/feature/nav-menu.html (1)
  • 16-16: The implementation for dynamically fetching the text for the navigation menu items based on the site's language setting is correct. This change ensures that the navigation menu is fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_includes/js/table-js.html (1)
  • 11-11: The conditional inclusion of language-specific DataTables configuration based on the lang variable is correctly implemented. This approach ensures that the DataTables functionality is fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_layouts/item/item-page-full-width.html (1)
  • 14-15: The dynamic fetching of the button text and its associated tooltip in the item page full-width layout based on the site's language setting is correctly implemented. This change ensures that the button and tooltip are fully localized, enhancing the user experience for non-English speakers and contributing to a more accessible website.
_layouts/item/item-page-base.html (1)
  • 13-14: The implementation of dynamic text for the link based on the site's language setting is a good approach for supporting multilingual content. Ensure that the site.lang variable is always correctly set and has a valid value to prevent any issues with missing translations.
_includes/item/3d-model-viewer.html (1)
  • 38-38: The dynamic loading of button text based on the site's language setting is a great enhancement for user experience in a multilingual context. Ensure that the translation data source is consistently maintained and covers all languages supported by the site.
_includes/cb/credits.html (1)
  • 3-12: Replacing hardcoded text with dynamic content from translation data in the Technical Credits section is a commendable effort towards supporting multilingual content. Ensure that translations are accurate and that the default fallbacks are appropriate for a broad audience.
_includes/feature/audio.html (1)
  • 34-34: Replacing the static error message with a dynamic, localized message based on the site's language setting is an excellent improvement for user experience. Ensure that the translation data source includes entries for all languages supported by the site to maintain consistency across different locales.
_includes/item/child/compound-item-download-buttons.html (1)
  • 10-16: Dynamically updating the text displayed on buttons based on the site's language setting is a significant improvement for multilingual support. Ensure that the translation data source is consistently maintained and accurately reflects all supported languages.
_includes/head/head.html (1)
  • 16-16: Updating the Content-Language meta tag to use a dynamic value from site.lang is a best practice for multilingual websites. Ensure that the site.lang variable is always correctly set and has a valid value to prevent any issues with incorrect language settings.
_includes/footer.html (1)
  • 33-39: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [36-48]

Updating the footer text to use dynamic translations for "built with" and "Last updated" based on the site's language setting is a valuable enhancement for multilingual support. Ensure that translations are accurate and comprehensive for all supported languages.

_includes/collection-banner.html (1)
  • 31-31: Updating the text displayed in the banner link to use dynamic translations based on the site's language setting is a good practice for enhancing multilingual support. Ensure that the translation data source is consistently maintained and accurately reflects all supported languages.
_layouts/browse.html (1)
  • 10-30: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [13-38]

Dynamically updating placeholders and button text on the browse page based on the site's language setting is an excellent enhancement for multilingual support. Ensure that the translation data source is consistently maintained and covers all languages supported by the site.

_includes/feature/pdf.html (1)
  • 38-39: Updating the error message for PDF rendering failures to be language-specific and providing a direct download link is a significant improvement for user experience in a multilingual context. Ensure that the translation data source includes entries for all languages supported by the site to maintain consistency across different locales.
_includes/index/data-download.html (1)
  • 4-25: Dynamically generating text for various data download button labels based on the site's language setting is an excellent enhancement for multilingual support. Ensure that the translation data source is consistently maintained and accurately reflects all supported languages.
_includes/item/browse-buttons.html (1)
  • 9-11: The implementation of dynamic language translations for the navigation buttons ("Previous", "Back to Browse", "Next") is correctly done using the site's language setting and translation data. This approach enhances the user experience for non-English speakers by providing localized navigation options. Ensure that the translation keys used here are correctly defined in the translation data files to avoid missing translations.
_includes/item/child/download-buttons.html (1)
  • 14-17: The dynamic updates to the button labels for "View on Timeline," "View on Map," "View on Vimeo," "View on YouTube," and "Link to Object" are correctly implemented using the site's language setting and translation data. This approach ensures that users can interact with the site in their preferred language, improving accessibility and user experience. Make sure the translation keys are accurately defined in the translation data files.
_includes/item/download-buttons.html (1)
  • 12-18: The dynamic updates to the button labels for viewing transcripts, timelines, maps, Vimeo, YouTube, and downloading content are correctly implemented using the site's language setting and translation data. This ensures a localized user experience across different parts of the site. It's important to verify that all translation keys are accurately defined in the translation data files to prevent any missing translations.
_layouts/search.html (2)
  • 8-23: The dynamic updates to the button text, modal titles, and descriptive text within the modal body for search options are correctly implemented using the site's language setting and translation data. This enhances the search functionality by providing localized instructions and options, improving the user experience for non-English speakers. Ensure that all translation keys are correctly defined in the translation data files.
  • 46-47: The footer text about search engines being powered by Lunr.js and the alternative Google CSE search option are also made translatable, which is a good practice for maintaining consistency in the localized user experience across the site. Verify that the translation keys for these sections are accurately defined in the translation data files.
_includes/feature/video.html (1)
  • 52-52: Replacing the static error message with a dynamic, language-specific one for the video element is a significant improvement for non-English speakers. This ensures that users receive localized feedback when their browser does not support the video element. Confirm that the translation key for the error message is correctly defined in the translation data files.
_includes/index/objects.html (1)
  • 5-33: The dynamic updates to the displayed text for different object types (image, audio, video, pdf, panorama, record, compound object, multiple) using translated strings are correctly implemented. This approach enhances the user experience by providing localized content, which is crucial for a multilingual site. Ensure that all translation keys for object types and the "View table" button are accurately defined in the translation data files.
_includes/feature/image.html (1)
  • 45-45: Updating the title attribute of the <img> tag to dynamically fetch a translation for the tooltip text based on the site's language setting is a thoughtful detail that enhances accessibility and user experience. This ensures that users receive localized tooltips, improving the overall usability of the site. Confirm that the translation key for the tooltip text is correctly defined in the translation data files.
_includes/js/browse-js.html (1)
  • 79-79: The implementation of dynamic translations for the "View Full Record" button is a significant enhancement for multilingual support. However, it's essential to ensure that the default filter is used correctly and that the translation keys exist in the site.data.translations structure. If the translation for a specific language is missing, it will gracefully fall back to the default text "View Full Record". This approach enhances the user experience by providing localized interface elements.
_includes/js/map-js.html (1)
  • 137-137: The dynamic translation for the "View Item" button in the map's popup template is a valuable addition for supporting multiple languages. This change ensures that users interacting with the map feature receive a localized experience, which is crucial for accessibility and user engagement. It's important to verify that the translation keys are correctly defined in the site.data.translations structure to ensure that the correct text is displayed for each language setting.
_includes/item/child/compound-item-modal-gallery.html (4)
  • 61-61: The update to the aria-label attribute for the SVG element with a dynamic translation for "Previous Item" is a thoughtful improvement for accessibility. This change ensures that screen readers can provide a localized description of the button's function, enhancing the user experience for non-English speakers and users with disabilities.
  • 64-64: The dynamic translation for the "Previous Item" text within the button is correctly implemented. This ensures that users navigating through the modal gallery receive localized text, improving the overall user experience across different languages.
  • 75-75: The dynamic translation for the "Next Item" text within the button is correctly implemented, similar to the "Previous Item" button. This consistency in implementing dynamic translations for navigation buttons within the modal gallery ensures a seamless and localized user experience.
  • 76-76: The update to the aria-label attribute for the SVG element with a dynamic translation for "Next Item" mirrors the improvement made for the "Previous Item" button. This consistency in enhancing accessibility through dynamic translations is commendable, ensuring that all users, regardless of language preference or disabilities, can navigate the modal gallery effectively.
_includes/data-download-modal.html (9)
  • 9-9: The implementation of dynamic button text using translation data is correct and follows best practices for multilingual support in Jekyll sites. This approach ensures that the button text is appropriately localized based on the site's language setting.
  • 15-15: The dynamic modal title using translation data is correctly implemented. It enhances the user experience by providing localized titles based on the selected language, adhering to best practices for multilingual websites.
  • 19-19: The dynamic description within the modal body is correctly implemented. This change ensures that the description is localized, improving accessibility and user experience for non-English speakers.
  • 24-29: The section for downloading complete metadata is correctly updated to include dynamic translations for the title, description, and button labels. This ensures that users can understand the options available in their preferred language, enhancing the site's multilingual capabilities.
  • 36-38: The dynamic translations for the metadata facets section are correctly implemented. This change improves the clarity of the options available for downloading metadata facets in the user's preferred language.
  • 45-48: The subject metadata section correctly uses dynamic translations for the title, description, and button labels. This ensures that the information is accessible and understandable to users in their preferred language, aligning with the goal of enhancing multilingual support.
  • 55-58: The location metadata section is correctly updated to include dynamic translations. This change ensures that users can access and understand the location metadata options in their preferred language, contributing to the site's multilingual capabilities.
  • 74-75: The timeline metadata section correctly uses dynamic translations for the title and description. This enhances the user experience by providing localized information about the timeline metadata options.
  • 82-84: The source code section is correctly updated to include dynamic translations for the title, description, and button label. This ensures that users can access the source code information in their preferred language, enhancing the site's multilingual support.

_includes/data-download-modal.html Show resolved Hide resolved
@maehr
Copy link
Member Author

maehr commented Feb 17, 2024

@koilebeit for bug hunting I set up https://maehr.github.io/collectionbuilder-csv/
There are many 😅

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 2afcc81 and b035a30.
Files ignored due to path filters (4)
  • _config.yml is excluded by: !**/*.yml
  • _data/translations.yml is excluded by: !**/*.yml
  • assets/lib/datatables/de.json is excluded by: !**/*.json
  • assets/lib/datatables/es.json is excluded by: !**/*.json
Files selected for processing (5)
  • _includes/js/browse-js.html (2 hunks)
  • _includes/js/table-js.html (1 hunks)
  • _layouts/browse.html (2 hunks)
  • _layouts/home-infographic.html (1 hunks)
  • _layouts/item/item-page-base.html (1 hunks)
Files skipped from review as they are similar to previous changes (5)
  • _includes/js/browse-js.html
  • _includes/js/table-js.html
  • _layouts/browse.html
  • _layouts/home-infographic.html
  • _layouts/item/item-page-base.html

_includes/cb/credits.html Outdated Show resolved Hide resolved
_includes/feature/pdf.html Outdated Show resolved Hide resolved
_includes/feature/video.html Outdated Show resolved Hide resolved
@koilebeit
Copy link

Thanks for the huge amount of work. I think there are still some translations missing, especially for buttons like the "view item" button on the carousel on the start page, and the search button on the search page.
Should the markdown pages in the pages folder also be translated? But this would not work via the translations.yml?

The markdown files will not be translated. Can you point to the code with the missing translations?

<button class="btn btn-primary dropdown-toggle" type="button" id="yearButton" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
Year
</button>

<h3><a href="#y{{ uniqueYears | first }}">{{ uniqueYears | first }}</a> to <a href="#y{{ uniqueYears | last }}">{{ uniqueYears | last }}</a></h3>

https://github.com/Stadt-Geschichte-Basel/forschung.stadtgeschichtebasel.ch/blob/5c1864d9bc66f833057853c4237cd6c40c988cf3/_layouts/search.html#L36C1-L38C22

{%- assign btn-text = include.btn-text | default: "View Item" -%}

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between b035a30 and d2d9774.
Files ignored due to path filters (1)
  • _data/translations.yml is excluded by: !**/*.yml
Files selected for processing (7)
  • _includes/cb/credits.html (1 hunks)
  • _includes/data-download-modal.html (1 hunks)
  • _includes/feature/nav-menu.html (1 hunks)
  • _includes/feature/pdf.html (1 hunks)
  • _includes/feature/video.html (1 hunks)
  • _includes/index/carousel.html (1 hunks)
  • _layouts/timeline.html (3 hunks)
Files skipped from review as they are similar to previous changes (5)
  • _includes/cb/credits.html
  • _includes/data-download-modal.html
  • _includes/feature/nav-menu.html
  • _includes/feature/pdf.html
  • _includes/feature/video.html
Additional comments: 4
_layouts/timeline.html (3)
  • 7-7: The introduction of the years variable to fetch the translation based on the site's language setting is a good enhancement for multilingual support. Ensure that the translations for 'Years' are correctly set up in the translations file for all supported languages.
  • 28-28: Updating the button label from "Year" to "years" improves consistency with the newly introduced years variable. This change aligns well with the objective of enhancing multilingual capabilities. However, ensure that the label is correctly capitalized according to the UI design standards or localization norms.
  • 39-39: Adjusting the date range display format to include links is a thoughtful UI improvement. It enhances user navigation within the timeline. Ensure that the uniqueYears variable accurately reflects the years available in the timeline data and that the links are functional and lead to the correct sections.
_includes/index/carousel.html (1)
  • 23-23: The update to dynamically fetch the 'btn-text' value based on the site's language setting is a significant improvement for multilingual support. This ensures that the carousel component's button label is correctly localized. Verify that the translations for "View Item" are correctly set up in the translations file for all supported languages and that the fallback to "View Item" works as expected in case a translation is missing.

@maehr
Copy link
Member Author

maehr commented Feb 21, 2024

Thanks for the huge amount of work. I think there are still some translations missing, especially for buttons like the "view item" button on the carousel on the start page, and the search button on the search page.
Should the markdown pages in the pages folder also be translated? But this would not work via the translations.yml?

The markdown files will not be translated. Can you point to the code with the missing translations?

<button class="btn btn-primary dropdown-toggle" type="button" id="yearButton" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
Year
</button>

<h3><a href="#y{{ uniqueYears | first }}">{{ uniqueYears | first }}</a> to <a href="#y{{ uniqueYears | last }}">{{ uniqueYears | last }}</a></h3>

https://github.com/Stadt-Geschichte-Basel/forschung.stadtgeschichtebasel.ch/blob/5c1864d9bc66f833057853c4237cd6c40c988cf3/_layouts/search.html#L36C1-L38C22

{%- assign btn-text = include.btn-text | default: "View Item" -%}

I fixed all. If you could use the inline comment feature next time https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request#starting-a-review GitHub can automate even more steps of the review.

@maehr
Copy link
Member Author

maehr commented Feb 23, 2024

I did some more bug hunting.

Screenshot_20240223-092000
Screenshot_20240223-092123

@maehr
Copy link
Member Author

maehr commented Feb 23, 2024

@koilebeit Ready for review. I fixed some more obvious bugs ;)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d2d9774 and 6f187da.
Files ignored due to path filters (1)
  • _data/translations.yml is excluded by: !**/*.yml
Files selected for processing (4)
  • _includes/item/image-gallery.html (1 hunks)
  • _includes/item/pdf-embed.html (1 hunks)
  • _includes/item/rights-box.html (1 hunks)
  • _includes/item/video-player.html (1 hunks)
Files skipped from review as they are similar to previous changes (4)
  • _includes/item/image-gallery.html
  • _includes/item/pdf-embed.html
  • _includes/item/rights-box.html
  • _includes/item/video-player.html

@maehr maehr mentioned this pull request Feb 26, 2024
7 tasks
@maehr maehr requested a review from mtwente February 26, 2024 10:19
@maehr maehr self-assigned this Feb 26, 2024
Copy link

@koilebeit koilebeit left a comment

Choose a reason for hiding this comment

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

It seems that the translation in the search bar in the navigation bar is not working:
image

_layouts/timeline.html Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 6f187da and 1e48ab8.
Files ignored due to path filters (1)
  • _data/translations.yml is excluded by: !**/*.yml
Files selected for processing (1)
  • _layouts/timeline.html (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • _layouts/timeline.html

@maehr
Copy link
Member Author

maehr commented Feb 28, 2024

It seems that the translation in the search bar in the navigation bar is not working: image

Fixed it

Screenshot 2024-02-28 at 11 20 36

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 1e48ab8 and 9493a25.
Files ignored due to path filters (1)
  • _data/translations.yml is excluded by: !**/*.yml
Files selected for processing (4)
  • _includes/item/download-buttons.html (1 hunks)
  • _includes/js/lunr-js.html (2 hunks)
  • _includes/nav-search-lunr.html (1 hunks)
  • _layouts/search.html (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • _includes/nav-search-lunr.html
Additional comments: 4
_includes/js/lunr-js.html (2)
  • 3-3: The introduction of the results_found variable to dynamically display the number of search results in the UI is a positive change for localization. However, ensure that the translation data is properly sanitized to prevent any potential security risks, especially since it's being inserted directly into the HTML.
  • 48-48: The dynamic update of the search results count using the results_found variable is correctly implemented. This enhances the user experience by providing localized feedback on the number of search results found. Good job on implementing this feature.
_includes/item/download-buttons.html (1)
  • 12-12: The dynamic update of button labels using translation data is a significant improvement for localization and user experience. Ensure that the translation data is properly sanitized to prevent any potential security risks, especially since it's being inserted directly into the HTML.

Also applies to: 15-15, 17-17, 19-19

_layouts/search.html (1)
  • 8-8: The updates to make button text, modal titles, and descriptive text within the modal body language-dependent are well-implemented. These changes significantly enhance the localization and user experience of the search functionality. Ensure that all translation data is properly sanitized to prevent any potential security risks.

Also applies to: 14-14, 15-15, 18-18, 20-23, 35-35, 37-37, 46-46, 47-47

_includes/js/lunr-js.html Show resolved Hide resolved
_includes/item/download-buttons.html Show resolved Hide resolved
_layouts/search.html Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 4

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 9493a25 and 060ec2d.
Files ignored due to path filters (1)
  • _config.yml is excluded by: !**/*.yml
Files selected for processing (8)
  • _includes/feature/audio.html (1 hunks)
  • _includes/feature/image.html (1 hunks)
  • _includes/feature/pdf.html (1 hunks)
  • _includes/feature/video.html (1 hunks)
  • _includes/index/carousel.html (1 hunks)
  • _includes/item/download-buttons.html (1 hunks)
  • _includes/js/map-js.html (1 hunks)
  • _layouts/timeline.html (3 hunks)
Files skipped from review as they are similar to previous changes (4)
  • _includes/feature/video.html
  • _includes/index/carousel.html
  • _includes/js/map-js.html
  • _layouts/timeline.html
Additional comments: 4
_includes/feature/audio.html (1)
  • 34-34: The dynamic error message for the audio element enhances the site's internationalization. This is a positive change that makes the site more accessible to users in different languages.
_includes/feature/pdf.html (1)
  • 38-38: The update to provide a localized error message and a direct download link for PDFs enhances user experience by catering to non-English speakers and offering an alternative when PDFs fail to render.
_includes/item/download-buttons.html (1)
  • 12-19: Updating button labels dynamically based on language settings is a significant improvement towards making the site more accessible and user-friendly for a global audience.
_includes/feature/image.html (1)
  • 45-45: Dynamically fetching a translation for the tooltip text based on the site's language setting is a thoughtful addition to the site's internationalization efforts.

_includes/feature/audio.html Show resolved Hide resolved
_includes/feature/pdf.html Show resolved Hide resolved
_includes/item/download-buttons.html Show resolved Hide resolved
_includes/feature/image.html Show resolved Hide resolved
@maehr maehr merged commit 531353e into Stadt-Geschichte-Basel:main Feb 28, 2024
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