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: send text formatting logic #1969

Merged
merged 6 commits into from
Jul 19, 2023

Conversation

alexandreferris
Copy link
Contributor


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

When sending text formatting for Header or Bold, it was a bit misplaced in the logic

  • # # was going for header
  • only ** was going for bold

Causes (Optional)

Logic was handling when isBold and not isHeader

Solutions

Change logic to handle from isHeader and not from isBold

@alexandreferris alexandreferris self-assigned this Jul 19, 2023
@alexandreferris alexandreferris requested review from a team, yamilmedina, vitorhugods, mchenani, Garzas and saleniuk and removed request for a team July 19, 2023 09:17
@github-actions
Copy link
Contributor

github-actions bot commented Jul 19, 2023

Test Results

506 tests  +6   505 ✔️ +6   25s ⏱️ +6s
  74 suites +1       1 💤 ±0 
  74 files   +1       0 ±0 

Results for commit 9a8c17d. ± Comparison against base commit 64ae349.

♻️ This comment has been updated with latest results.

Copy link
Member

@vitorhugods vitorhugods left a comment

Choose a reason for hiding this comment

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

MessageCompositionHolder could get some new tests ;)

@alexandreferris alexandreferris disabled auto-merge July 19, 2023 09:43
@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #1969 (9a8c17d) into develop (a44c67b) will increase coverage by 0.24%.
The diff coverage is 100.00%.

@@              Coverage Diff              @@
##             develop    #1969      +/-   ##
=============================================
+ Coverage      38.24%   38.49%   +0.24%     
- Complexity       886      892       +6     
=============================================
  Files            287      287              
  Lines          11313    11313              
  Branches        1449     1449              
=============================================
+ Hits            4327     4355      +28     
+ Misses          6559     6530      -29     
- Partials         427      428       +1     
Impacted Files Coverage Δ
.../messagecomposer/state/MessageCompositionHolder.kt 28.22% <100.00%> (+13.39%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64ae349...9a8c17d. Read the comment docs.

@github-actions
Copy link
Contributor

Build (dev-debug) available here. Scroll down to Artifacts!

@github-actions
Copy link
Contributor

Build (beta-debug) available here. Scroll down to Artifacts!

Copy link
Member

@vitorhugods vitorhugods left a comment

Choose a reason for hiding this comment

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

👨🏻‍🍳🤌🏻

@alexandreferris alexandreferris added this pull request to the merge queue Jul 19, 2023
@github-actions
Copy link
Contributor

Build (beta-debug) available here. Scroll down to Artifacts!

@github-actions
Copy link
Contributor

Build (dev-debug) available here. Scroll down to Artifacts!

Merged via the queue into develop with commit 49844b1 Jul 19, 2023
@alexandreferris alexandreferris deleted the fix/send_text_formatting_logic branch July 19, 2023 10:37
@AndroidBob
Copy link
Collaborator

Build 514 succeeded.

The build produced the following APK's:

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.

5 participants