New design reader bottom sheet - step 2 #11184
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 test the changes on this Pull Request by downloading the APK here. |
…10887-new-design-reader-bottom-sheet-step2
develric
added
the
[Status] Needs Design Review
…10887-new-design-reader-bottom-sheet-step2 # Conflicts: # WordPress/src/main/res/values/styles.xml
3 tasks
osullivanchris
approved these changes
Jan 30, 2020
3 tasks
…10887-new-design-reader-bottom-sheet-step2
develric
added
[Status] Needs Code Review
and removed
[Status] Needs Design Review
khaykov
approved these changes
Feb 1, 2020
| } | ||
| it.setTextColor(ContextCompat.getColor( | ||
| parent.context, | ||
| if (filter.isSelected) |
There was a problem hiding this comment.
This is super minor, but can we add curly brackets to if/else here? It could prevent some silly issues in the future :)
There was a problem hiding this comment.
👍changed here f97bbf9
|
|
||
| val blog = site.blog | ||
|
|
||
| this.itemUrl.text = if (blog.hasUrl()) { |
There was a problem hiding this comment.
Maybe we can switch to when for better readability here?
There was a problem hiding this comment.
TIL! I was actually not aware of this when without parameter form 😬, thanks 😊! Changed here f97bbf9
|
Hey @khaykov ! Thanks for looking into this one 🙇♂️! I made the changes based on your comments. Let me know if it's ok 😊. |
khaykov
approved these changes
Feb 3, 2020
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.
Fixes #11009 and partially #10887
NOTE: this requires PR #11183 merged first into develop
Notes
The feature is active in the zalpha flavor.
This PR covers the following:
SITESsection to present the list items with both the site title and a URL. In this way it's easier to manage cases where the site title is empty or equal since the match and selection is done on both.To test
Test case 1: SITES tab shows site title and URL
Test case 2: SITES tab shows and select Untitled sites correctly
Test case 3
Since we made modifications to the
add_content_bottom_sheetthat is used also in the main create fab bottom sheet, makes sense to tap on the main FAB in My Site and smoke test the bottom sheet.PR submission checklist:
note: not adding accessibility here since there will be a dedicated audit of the feature to close ticket #10962
RELEASE-NOTES.txtif necessary.