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.

Sign up for GitHub

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

Jump to bottom

Fix to prevent Korean language from being broken in PDF #1008

Merged
merged 1 commit into from
Nov 24, 2023
Merged

Fix to prevent Korean language from being broken in PDF #1008

merged 1 commit into from
Nov 24, 2023

Conversation

MyunghyunNero
Copy link
Contributor

Description

close #1005

AS IS

Korean is broken in PDF.
image

TO BE

image

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation update
  • Refactoring, Maintenance
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@jongwooo jongwooo added the bug Something isn't working label Oct 10, 2023
jongwooo
jongwooo suggested changes Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@jongwooo jongwooo left a comment

Choose a reason for hiding this comment

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

@MyunghyunNero Please remove the .DS_Store files.

70825
70825 reviewed Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@70825 70825 left a comment

Choose a reason for hiding this comment

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

제가 알기로 글꼴을 적용하는 부분은 작년에 아래 링크에 있는 report html에 base64로 적용했었는데요.
지금은 나눔 고딕 글꼴 파일이 추가되어서 질문 드려요
report html에 있는 글꼴을 제거하거나, 나눔 고딕을 base64로 교체해야 할 것 같은데 어떻게 생각하시나요?

작년 PR 링크
report html에 글꼴이 적용된 위치 코드

@70825 70825 added bug fix [PR] Fix the bug and removed bug Something isn't working labels Oct 10, 2023
@MyunghyunNero
Copy link
Contributor Author

MyunghyunNero commented Oct 10, 2023 •
edited
Loading

@70825
제가 해당 이슈 해결하면서 Itext에서 pdf 변환 중 한글 적용을 하기 위해서는 한글지원하는 글꼴을 적용해야한다고 하더라구요. 그래서 일단 나눔고딕을 이용해서 진행을 했습니다!
다빈님이 말씀해주신 방향으로 base64 글꼴을 찾아봤는데 관련 ttf파일은 없구 디코딩/인코딩 부분만 나와서 일단 적용은 못했습니다!
두번쨰로 report html 글꼴을 제거 할 경우에는

AS-IS

image

TO BE

image

모든 html 부분이 나눔 고딕으로 덮어씌워지는 것 같습니다!

그래서 html font 부분을 제거하고 htmlConverter에서 글꼴을 나눔고딕 말고 다른 부분으로 수정해보겠습니다!

70825
70825 reviewed Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@70825 70825 left a comment

Choose a reason for hiding this comment

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

글꼴 하나만 적용되는게 확실히 깔끔해보이네요 👍👍
반영되면 다시 review request 보내주세요

@MyunghyunNero
Copy link
Contributor Author

@70825
한글 글꼴 중 Bold 로 통일 했을 때
image

이런 식으로 나오고 있습니다!

70825
70825 suggested changes Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@70825 70825 left a comment

Choose a reason for hiding this comment

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

고생하셨어요~ 간단하게 코멘트 남겼습니다 !
글꼴이나 bold의 경우엔 다른 분이 리뷰를 진행해주실 것 같아요
그리고 리뷰한 것 이외에 콤마 다음에 띄어쓰기를 하지 않은 간단한 컨벤션 부분도 같이 수정해주세요~

src/main/java/oss/fosslight/util/PdfUtil.java Outdated
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
HtmlConverter.convertToPdf(html, outputStream);
HtmlConverter.convertToPdf(html, outputStream,properties);
Copy link
Collaborator

@70825 70825 Oct 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

기능별로 줄바꿈을 사용하여 코드들을 구분하면 읽기 더 편할 것 같아요

src/main/java/oss/fosslight/util/PdfUtil.java Outdated
@@ -48,9 +51,15 @@ public static PdfUtil getInstance() {
}
return instance;
}
public static ByteArrayInputStream html2pdf(String html) {
public static ByteArrayInputStream html2pdf(String html) throws IOException {
String Font = "src/main/resources/static/NanumGothicBold.ttf";
Copy link
Collaborator

@70825 70825 Oct 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

해당 부분은 카멜 케이스로 변수 네이밍을 하지 않은 이유가 있을까요?
그리고 이 부분은 파일 경로가 바뀌지 않으니 상수로 지정해도 무방해보입니다.

@MyunghyunNero
Fix PDF Korean Broken Issue
7503728 
Signed-off-by: MyunghyunNero <mh1kim0910@gmail.com>
@github-actions GitHub Actions
Copy link

This PR has been automatically marked as stale because it hasn't had any recent activity.
It will be closed in 5 days if no further activity occurs.

@github-actions github-actions bot added the stale label Nov 13, 2023
@github-actions GitHub Actions
Copy link

This PR was closed because it has been inactive for 5 days since being marked as stale.

@github-actions github-actions bot closed this Nov 19, 2023
@soimkim soimkim reopened this Nov 24, 2023
@soimkim soimkim removed the stale label Nov 24, 2023
@soimkim soimkim merged commit 216a55c into fosslight:develop Nov 24, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jongwooo jongwooo jongwooo requested changes

@70825 70825 70825 requested changes

Assignees

@MyunghyunNero MyunghyunNero

Labels
bug fix [PR] Fix the bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Korean is broken in PDF
4 participants
@MyunghyunNero @jongwooo @70825 @soimkim