Skip to content

feat: Add basic diacritic characters, European mostly (#867) #1301

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

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

yarda
Copy link
Contributor

@yarda yarda commented May 14, 2025

Fixes #867

Changes

  • Added basic diacritic characters, European mostly

Screenshots / Recordings

NA

Checklist:

  • No hard coding: I have used resources from constants.dart without hard coding any value.
  • No end of file edits: No modifications done at end of resource files.
  • Code reformatting: I have reformatted code and fixed indentation in every file included in this pull request.
  • Code analyzation: My code passes analyzations run in flutter analyze and tests run in flutter test.
    I am not sure how to do this :)

Summary by Sourcery

New Features:

  • Support basic European diacritic characters in DataToByteArrayConverter by adding their byte‐array mappings

Copy link
Contributor

sourcery-ai bot commented May 14, 2025

Reviewer's Guide

This PR enhances the DataToByteArrayConverter by populating it with basic European diacritic characters—both uppercase and lowercase—while also cleaning up the existing mapping literal for consistent formatting.

File-Level Changes

Change Details Files
Added basic European diacritic characters to the bytearray mapping
  • Added mapping entries for uppercase accented letters (Á, É, Í, Ó, Ú, Ç, Ñ, Č, Š, Ř, Ž)
  • Added mapping entries for lowercase accented letters (á, é, í, ó, ú, ç, ñ, č, š, ř, ž)
lib/bademagic_module/utils/data_to_bytearray_converter.dart
Reformatted mapping literal for style consistency
  • Inserted trailing commas after map entries
  • Aligned indentation and spacing across the map
lib/bademagic_module/utils/data_to_bytearray_converter.dart

Assessment against linked issues

Issue Objective Addressed Explanation
#867 Correctly render non-latin characters like 'Šutr' on both the Android preview and the badge.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @yarda - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@yarda
Copy link
Contributor Author

yarda commented May 14, 2025

These are very simple characters. I tried it looks as close as possible to the original font, feel free to improve it :). I hacked very simple editor, that can help with this task :). I hope somebody could find it useful :) https://github.com/yarda/BMPFontDesigner

@yarda
Copy link
Contributor Author

yarda commented May 14, 2025

I also added empty lines for better readability of the characters table and to group them.

@yarda
Copy link
Contributor Author

yarda commented May 14, 2025

Please do not merge it yet. I used AI to gather list of characters needed, and it seems it forgot "ďĎťŤ" characters which are used e.g. in Czech language. I will add them and update the PR.

@yarda yarda force-pushed the basic-diacritic-chars-add branch from 55300ff to f82490f Compare May 14, 2025 20:30
@yarda
Copy link
Contributor Author

yarda commented May 14, 2025

Also added "ěĚďĎňŇťŤ" characters. So at least the Czech language, which I am the most interested in, seems 100% covered now.

@AsCress AsCress self-requested a review May 15, 2025 13:53
@AsCress
Copy link
Contributor

AsCress commented May 15, 2025

Some of the added characters:

Screenshots

@yarda Generally all of them look good to me, all fit within the size of the badge. Some of them had to be deformed perhaps to fit in the badge ? But in general, these look good.

@yarda yarda force-pushed the basic-diacritic-chars-add branch 2 times, most recently from 8e4d0c8 to 90e29a4 Compare May 15, 2025 16:13
@yarda
Copy link
Contributor Author

yarda commented May 15, 2025

I also improved the 'Å' character a bit.

@yarda
Copy link
Contributor Author

yarda commented May 15, 2025

It still isn't complete character set for all European languages alphabet, but I think it is a good start.

@AsCress
Copy link
Contributor

AsCress commented May 15, 2025

@yarda The CI still fails because of the code not being formatted. Just run dart format . in the root of your project and commit the changes.

@yarda
Copy link
Contributor Author

yarda commented May 15, 2025

@yarda The CI still fails because of the code not being formatted. Just run dart format . in the root of your project and commit the changes.

The recent force push added missing CRs to the empty lines which was probably the cause of the failure. It is according to the output of the dart format . of my added changes. The dart format . also changed formatting of the code I didn't add, but if you need it, I can reformat the whole source file with it and update the PR.

@AsCress
Copy link
Contributor

AsCress commented May 15, 2025

@yarda Just tried it myself on your branch. It only formats the file that you've changed and removes the empty lines.
Screenshot 2025-05-15 215020

@yarda
Copy link
Contributor Author

yarda commented May 15, 2025

@yarda Just tried it myself on your branch. It only formats the file that you've changed and removes the empty lines. !

NP, I will drop the empty lines. I probably used different dart formatter :)

@yarda yarda force-pushed the basic-diacritic-chars-add branch from 90e29a4 to 0900995 Compare May 15, 2025 16:23
@yarda
Copy link
Contributor Author

yarda commented May 15, 2025

It seems 2 tests failed, but I don't know why.

@AsCress
Copy link
Contributor

AsCress commented May 15, 2025

It seems 2 tests failed, but I don't know why.

Yeah, noticed that as well. However, I may not be able to assist with this, @Jhalakupadhyay might be able to help.

@AsCress
Copy link
Contributor

AsCress commented May 15, 2025

However, this is good to go once the tests pass.

@adityastic
Copy link
Collaborator

Mind updating the tests as well?

@yarda yarda force-pushed the basic-diacritic-chars-add branch from 0900995 to e645a37 Compare May 15, 2025 23:44
@yarda
Copy link
Contributor Author

yarda commented May 15, 2025

Mind updating the tests as well?

I tried to fix the tests.

Copy link
Contributor

github-actions bot commented May 16, 2025

Build Status

Build successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/15421999639/artifacts/3251583181.

Screenshots (Android)

Screenshots (iPhone)

Screenshots (iPad)

@yarda yarda force-pushed the basic-diacritic-chars-add branch from e645a37 to 126689c Compare May 16, 2025 06:59
@yarda
Copy link
Contributor Author

yarda commented May 16, 2025

In the second version of the test fix I improved the test coverage.

@adityastic adityastic enabled auto-merge (squash) June 2, 2025 09:29
@adityastic adityastic force-pushed the basic-diacritic-chars-add branch from 126689c to f156d31 Compare June 2, 2025 09:38
@adityastic adityastic force-pushed the basic-diacritic-chars-add branch from f156d31 to d52d3ee Compare June 3, 2025 15:52
@adityastic adityastic merged commit eb6ec45 into fossasia:flutter_app Jun 3, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please fix non latin support
3 participants