-
Notifications
You must be signed in to change notification settings - Fork 246
Adds an arrow in front of a expandable stream to expand and contract … #469
base: master
Are you sure you want to change the base?
Conversation
Automated message from Dropbox CLA bot @abhaymaniyar, it looks like you've already signed the Dropbox CLA. Thanks! |
@Sam1301 Please review and help me to resolve merge conflicts. |
55c0d32
to
c0193c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @abhaymaniyar I have left some comments.
<!--android:textSize="14sp"--> | ||
<!--android:textStyle="bold" />--> | ||
<!--</LinearLayout>--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this comments if there are not being used anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @saketkumar95! :)
@@ -30,7 +30,7 @@ public StreamSpan(String streamId, int color) { | |||
@Override | |||
public void onClick(View widget) { | |||
Context context = widget.getContext().getApplicationContext(); | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra newline, remove this too. :)
c0193c2
to
a97409e
Compare
Thank you @saketkumar95 :). Can you please help me with the conflicts here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abhaymaniyar So this has one problem currently in ExpandableStreamDrawerAdapter.java we are getting the child's of the streamsDrawer from the current message cache in the DB, hence it's highly possible that we have no topics for that stream, and still the arrow indicator shows up,
Hence it would be better to not show the arrowHead for the streams which we don't have any topics!
Thanks for reviewing @kunall17! Is there any variable in the code base which stores the number of topics in a stream? |
@kunall17 I am trying to implement what you said but there is a problem. I get the number of subjects in a stream only when I click on the stream. How can I get the number of subjects in a stream on launch of MainActivity? |
Yeah, true it only get's calculated when the expansion of the expandableListItem is done, so should we have arrow in all? |
Great! I can do that. |
abc75bb
to
d2714f5
Compare
@abhaymaniyar, your pull request has developed a merge conflict! Please review the most recent commit (f791888) for conflicting changes and resolve your pull request's merge conflicts. |
…that stream.
Summary of changes
When a user clicked on a stream in the stream pane, it narrows down the stream. Now if a user clicks on the stream it expands the stream and shows the topics that the stream contains.
Screenshots or a brief video showing the change in action:
Please make sure these boxes are checked before submitting your pull request - thanks! Guide
Code is formatted.
Run the lint tests with
./gradlew lintDebug
and make sure BUILD is SUCCESSFULIf new feature is implemeted then it is compatible with Night theme too.
Commit messages are well-written.