Reader Detail Updates (Post Header) #13042
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
10 tasks
|
You can test the changes on this Pull Request by downloading the APK here. |
ashiagr
added
the
Part of a WIP Feature
…00-rip3-details-updates-part-ii
3 tasks
ashiagr
changed the base branch from
issue/12900-rip3-details-updates-part-i
to
issue/12900-reader-post-header-kotlin
3 tasks
ashiagr
changed the base branch from
issue/12900-reader-post-header-kotlin
to
issue/12900-rip3-details-updates-part-ii-split-kotlin
ashiagr
force-pushed
the
issue/12900-rip3-details-updates-part-ii
branch
from
3a356da to
02cb442
Compare
…into issue/12900-rip3-details-updates-part-ii # Conflicts: # WordPress/src/main/res/values/dimens.xml
3 tasks
ashiagr
changed the base branch from
issue/12900-rip3-details-updates-part-ii-split-kotlin
to
issue/12900-rip3-details-updates-part-ii-split-uistate
… into issue/12900-rip3-details-updates-part-ii # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/reader/discover/ReaderPostUiStateBuilder.kt
malinajirka
reviewed
Oct 6, 2020
There was a problem hiding this comment.
Thanks @ashiagr! All three PRs look good overall. I've encountered just one UI glitch.
When the site title is empty, the header positioning breaks.
Example Post: https://wp.me/pVHyx-t4
It might be related to one of the child PRs - I've tested all the changes altogether in this PR.
WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostDetailFragment.kt
Outdated
Show resolved
Hide resolved
|
Thanks @malinajirka ! I've addressed PR review comments.
This issue exists on develop as well. I'll create a separate PR targeting develop. |
3 tasks
malinajirka
approved these changes
Oct 7, 2020
Base automatically changed from
issue/12900-rip3-details-updates-part-ii-split-uistate
to
issue/12900-rip3-detail-updates-main
October 12, 2020 05:21
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Task: #12900
This PR refactors post details header view with following changes:
Child PRs:
PR #13060 - Kotlin conversion
PR #13061 -
uiStateextracted for blog section and follow buttonuiStateextracted in PR Reader Detail Updates (Post Header - Extract UiState) #13061 03db5a1reader_include_post_detail_content.xml8790672Merge instructions
To test
Note
Most of the changes are due to extraction and reuse of blog section from- Reader Detail Updates (Post Header - Kotlin Conversion) #13060, Reader Detail Updates (Post Header - Extract UiState) #13061 split from this PRReaderPostUiState(2c6fac5, 8dbc4f8). I can split this PR into smaller ones if anyone finds it difficult to review.PR submission checklist:
RELEASE-NOTES.txtif necessary.