Skip to content

PR review feedback for threads - #927

Merged
tomholub merged 2 commits into
masterfrom
feature/issue-920
Nov 5, 2021
Merged

PR review feedback for threads#927
tomholub merged 2 commits into
masterfrom
feature/issue-920

Conversation

@ivan-ushakov

@ivan-ushakovivan-ushakov commented Nov 5, 2021

Copy link
Copy Markdown
Contributor

close#920


Tests(delete all except exactly one):

  • Does not need tests (refactor only, docs or internal changes)

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@ivan-ushakov

Copy link
Copy Markdown
ContributorAuthor

@Kharchevskyi
Since I'm not good in ADK but I did some changes in your code, I added you to this PR

@ivan-ushakov

Copy link
Copy Markdown
ContributorAuthor

Before:

Simulator Screen Shot - iPhone 11 Pro - 2021-11-05 at 12 56 36

After:

Simulator Screen Shot - iPhone 11 Pro - 2021-11-05 at 13 25 18

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good - see comment

Comment threadFlowCrypt/Controllers/Inbox/InboxViewDecorator.swift
@tomholub

Copy link
Copy Markdown
Collaborator

But indeed the layout is somewhat broken. Anton is off work until next week. Maybe @sosnovsky or @ekievsky can help.

@sosnovsky

Copy link
Copy Markdown
Collaborator

I'm here, should I try to fix these layout issues?

@ivan-ushakov
ivan-ushakov requested review from sosnovsky and removed request for KharchevskyiNovember 5, 2021 10:44
@ivan-ushakov

Copy link
Copy Markdown
ContributorAuthor

I'm here, should I try to fix these layout issues?

Please wait for my fix and do a review

@sosnovsky

Copy link
Copy Markdown
Collaborator

Looks good now 👍

@tomholub
tomholub enabled auto-merge (squash) November 5, 2021 11:08
@tomholub
tomholub merged commit 7cb5a92 into masterNov 5, 2021
@tomholub
tomholub deleted the feature/issue-920 branch November 5, 2021 11:32
Sign up for freeto 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.

PR review feedback for threads - fix download loader, n. of threads render

3 participants

@ivan-ushakov@tomholub@sosnovsky