Fix an issue (hard) OpenIntents
Status: Closed Time to complete: 336 hrs Mentors: Peli, Friedger Müffke, Randy McEoin, aap, Manuel R. Ciosici Tags: Android, Java

The task is to fix one of the issues from the list of issues.

Your solution should consist of a patch (instructions) based on the latest version of OI Safe. If you dont have it yet, you first need to download and install the Android SDK (instructions). Follow these instructions to build OI applications.

  • Difficulty: hard
  • Time limit: 14 days
  • Prerequisites: Download the Android SDK (instructions)
  • Requirements for completed entry:
    • IMPORTANT: Ask first whether the issue you want to work on is still available and whether you should choose an easy, medium, or hard task.
    • Send in a patch with your code modifications (instructions).
    • The code must follow our StyleGuide.
    • The code must fix the issue.
    • Depending on the number of stars of the issue, select an easy (1 star), medium (2 stars), or hard (3 or more stars) task.
Uploaded Work
File name/URL File size Date submitted
notesicon.svn.patch 6.7 KB December 04 2011 23:35 UTC
notesicon.svn.patch 4.2 KB December 17 2011 15:46 UTC
notesicon.svn.patch 7.2 KB December 28 2011 04:25 UTC
notesicon.svn.patch 7.2 KB December 28 2011 04:25 UTC
notesicon.svn.patch 7.2 KB December 28 2011 04:25 UTC
Comments
Shuhao on December 4 2011 16:29 UTC Task Claimed

I would like to work on this task.

Shuhao on December 4 2011 16:30 UTC Issue 340

I would like to work on issue 340: http://code.google.com/p/openintents/issues/detail?id=340


Currently having trouble finding the code that displays the note... i'll keep trying.

Shuhao on December 4 2011 16:35 UTC Problem traced

It looks like the note icon is set to display on all items with notes. However, it seems that when it's beyond 1 line, the note icon is pushed off screen. I might need to reposition the note icon and the textview. (Also, shouldn't there be a context menu button for items with notes as well?)

aap on December 4 2011 16:55 UTC Task Assigned

This task has been assigned to Shuhao. You have 336 hours to complete this task, good luck!

aap on December 4 2011 16:59 UTC Assigned

I look forward to reviewing your fix. Feel free to discuss your findings and/or approach in the developer forum


 

Shuhao on December 4 2011 23:35 UTC Ready for review

The work on this task is ready to be reviewed.

Shuhao on December 4 2011 23:35 UTC Task done

That's gotta be one of the more difficult task i've done.. After all, UI code is my worst nightmare.

aap on December 5 2011 01:54 UTC Testing

Interesting. I will definitely test that a bit before commenting on the actual change.


Is the .classpath change intentional, or just something Eclipse did? It doesn't seem related to the task.


 

Shuhao on December 5 2011 01:55 UTC .classpath

.classpath changed? I tried to make sure it didn't.. and yes it's something Eclipse did. (Someone should really ignore classpath..)

aap on December 5 2011 02:25 UTC .classpath

It's a good idea to read the patch file before posting it... if you had done that you would have seen the .classpath change. I also tend to use "git status" and manually git add just the changed files I actually want to commit. Anyway, I'm off to test the actual fix now...

Shuhao on December 5 2011 02:26 UTC .classpath

Yeah. i did all of that.. maybe i missed something this time..


 


oh well.

aap on December 5 2011 02:38 UTC Needs work

You're right, this is a challenging one. Please test with some items that have different combinations of price, quantity, and priority. With your change those fields are overlapping the note icon or the item name.

aap on December 5 2011 02:38 UTC Task Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Shuhao on December 5 2011 02:55 UTC LinearLayout

I think the way to do it is to wrap everything from quantity to the has_note in a LinearLayout

aap on December 5 2011 03:13 UTC Maybe, but...

It probably would work, but the author of this post seems to think that using LinearLayout in a RelativeLayout is bad for performance. Perhaps we just need to rebalance the dependencies? In other words, imagine an item which has all the fields populated, and declare the dependencies of each field based on that arrangement. The trick mentioned there with android:layout_alignWithParentIfMissing="true" might also help us.

Shuhao on December 5 2011 03:18 UTC LinearLayout

There are more problems with RelativeLayout. For example, .setVisibility(View.GONE) won't work.


1 LinearView won't really hurt that much, as Google points out: http://developer.android.com/resources/tutorials/views/hello-linearlayout.html


I dunno though, so far I don't have much luck making something align to the right side of the screen with a LinearView (maybe someone could help me with that)


 

aap on December 5 2011 03:27 UTC View.GONE

Please elaborate on the problem with View.GONE. We do use GONE in ShoppingListView.java for some items in the RelativeLayout. 

Shuhao on December 5 2011 03:29 UTC View.GONE

http://stackoverflow.com/questions/3279081/issue-with-relativelayout-when-view-visibility-is-view-gone


I was confused too, but if you look at my previous implementation, you would see that even if there's no note, there's still a giant gap to the right of the screen, which means that the View.GONE setting was ineffective.

aap on December 5 2011 03:35 UTC android:gravity

As for making something appear on the right side of a LinearLayout, for the fields you mentioned it may not be required*, but the way to do it is probably with the android:gravity attribute.


* At least for former HandyShopper users it is more natural to see the note right next to the text as though it were just another character of the note. I guess we probably can't do that when the item name wraps to two lines but when it fits on one line it could. So I don't see the need to align the has_note to the right. Or were you thinking of the price? I was assuming that the price would remain outside your LinearLayout.

aap on December 5 2011 03:41 UTC minding the gap

I think the solution for the gap on the right is probably the alignWithParentIfMissing attribute. But it does sound like that would only help on the edge of the layout, not in the middle. The solution in the middle might be to populate the "GONE" views with empty strings instead of making them GONE. I suspect that the behavior in this regard is different on different Android versions,

aap on December 5 2011 03:45 UTC LinearLayout

I am somewhat sensitive to the performance issue because my main list has over 700 items, which makes things bog down in Pick mode. But you're right that some of the positioning issues would seem to be much easier to deal with in a LinearLayout. So if you want to try it that way I won't rule it out before testing your solution. I suppose there is even a chance that with the number of relationships we currently have the LinearLayout might even turn out to be faster.

Shuhao on December 5 2011 03:46 UTC The Gone view is the has_note image

Yes the price remains outside of the linear view. Everything that has large text will be in the same linear view. However, I don't know how it would work (multiple horizontal textview)


I'll need to investigate tomorrow. 


My Android versions are 2.2 and 2.3.7 and they have the same behaviour.


I still think the notes shouldn't be after the last character for 2 reasons. a) We can't actually place it there if wraps. b) The notes really isn't a character, and why do we need to copy HandyShopper? :p


In my opinion, it looks prettier when everything aligns.. but that's just my opinion.

Shuhao on December 5 2011 03:47 UTC The code that adds the items right now?

I'm having trouble finding the code that adds the items to the list. I'll need to add a line that adds the linear layout, i believe. Maybe you could point me towards the right direction?


Thanks

aap on December 5 2011 04:03 UTC adding items

The LinearLayout doesn't need to be added in the code, just in those same item row XML files you modified in your first attempt. The code that fills in the fields inside the LinearLayout might need adjusting to find them... I'm not sure. 


The list items rows are populated directly from a database query by a SimpleCursorAdapter in ShoppingItemsView.java . For each item which fits on the screen, the LayoutInflater is called to generate the views from one or the other of those two xml files. See how layout_row is chosen and then passed as an argument to mSimpleCursorAdapter.


 

aap on December 5 2011 04:10 UTC prettier when everything aligns?

If we really wanted everything to align, even when some fields exist and some don't, probably we would use a table view. But that would mean that some space is reserved for the note icon even for items which don't have a note. The current layout makes better use of space within each item. 

Shuhao on December 5 2011 04:13 UTC GONE

That should be where GONE come into play. Under a linearview (not sure about tableview), setting note's visibility to GONE will free up that space, the element next to it (which is the name of the item), will take up that space instead of leaving it blank.


Also, i don't think it's possible to get the note icon back when it wraps around a line. It seems to be pushed off screen.

aap on December 5 2011 05:12 UTC developer forum

By now it's clear that we should have moved this discussion to the developer forum hours ago. Let's continue the discussion there.

Shuhao on December 17 2011 15:46 UTC Ready for review

The work on this task is ready to be reviewed.

Shuhao on December 17 2011 15:48 UTC Fixed, kind of

Since the layout before is already kind of messed up, this one simply fixed the notesicon... as to the general layout issue, that has to be resolved with more discussions...

Peli on December 17 2011 21:16 UTC Hm

With this new patch, it is very likely that "price" and "note" fields overlap. (If there is a short item name, a price, and a note, and nothing else, then they will overlap.)


The question is whether this is a likely scenario, and whether having the note guaranteed on screen is worth introducing this new oddity.


Aap, would you be happy with putting the Note icon either to the left most (after check box but before item name) position or to the right-most position (after price), so that it can never overlap with any other items?


By the way, note that a new bug has been revealed with a recent patch for OI Notepad, which crashes OI Notepad if one presses on the note icon.


http://code.google.com/p/openintents/issues/detail?id=434


So for now it is better not to update OI Notepad or use the latest pubished version.

aap on December 18 2011 01:57 UTC left would be OK, but let's try this first:

When you say "and nothing else" I assume what you really mean is that any configuration that results in displaying the note icon, the price, and no tags would cause the overlap.


As a temporary workaround for issue 340, my wife made me give her a custom build which puts the note on the left. So maybe I could live with that. Actually I just had temporarily tried it that way and I reverted it because it wasn't quite right -- but she had seen it and asked me to put it back that way until we figure out the real fix.


I think it would be better, though, if we would leave the note on the right, but programmatically add "toRightOf item name" to the price field when the item in question is not going to display a tag either because there is none or because tag display is turned off. This will fix overlap even in the case where there is no note but the item name is long enough to reach near the right edge.


I only hope it won't make the price bump off the side of the screen...

aap on December 18 2011 01:57 UTC Task Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

aap on December 18 2011 01:57 UTC Deadline extended

The deadline of the task has been extended with 2 days and 0 hours.

aap on December 18 2011 02:26 UTC bindView()

I assume the programmatic manipulation of the price would be done in ShoppingItemsView.java in the bindView() function. When we already have a handle to the price view (line 351) we could call v.getLayoutParams(), cast the returned value to RelativeLayout.LayoutParams, and call its addRule method to add the toRightOf constraint. 


If that does cause the price to flip off the screen, might be worth trying to programmatically add toLeftOf to the item description instead.


I added two days because the clock was running out but you have been trying... it's just a tough one. If you think you will need more time beyond those two extra days, let us know how much you think you will require and we will consider it.

Melange on December 21 2011 00:47 UTC Task Reopened

Melange has detected that the final deadline has passed and it has reopened the task.

Shuhao on December 21 2011 00:47 UTC Task Claimed

I would like to work on this task.

Peli on December 21 2011 00:51 UTC Task Assigned

This task has been assigned to Shuhao. You have 336 hours to complete this task, good luck!

Shuhao on December 27 2011 22:15 UTC Overlap?

There is no overlap with the notes, tags, and prices with my patch


Here's a screenshot: http://i.imgur.com/THS2q.png


Sorry for the late reply... was pretty busy over the last 2 weeks.


 

aap on December 28 2011 03:44 UTC Yes, overlap

Actually your patch was accidentally included on trunk for a little while, and I noticed its presence due to the overlap. Try with price but no tags, or with tag display turned off... the note icon and the price overlap. Might also need to use a smaller font size to see it.

Shuhao on December 28 2011 04:25 UTC Ready for review

The work on this task is ready to be reviewed.

Shuhao on December 28 2011 04:25 UTC Got it working

Finally was able to get it working and I hope this would be it.. I can't find any other problems myself.

aap on December 29 2011 00:06 UTC extra row with no tags?

Need to test when I get home... but from reading the xml it looks like it would make an extra mostly-empty row which would not have been there without this patch, in the case where an item has no tags (or tag display is turned off) but has price... perhaps even if it has no note. Am I reading it correctly?

Shuhao on December 30 2011 20:53 UTC Progress?

If I understood you correctly, I'm pretty sure this is what it does.


Any progress on testing this?

Peli on December 30 2011 22:19 UTC Looked at it

I just tested your patch. It indeed solves the problem, but in a way that would waste screen space for most users.


I think (at least according to me) the most common use case is simply an item name and a price (without tag or note), and in this case 2 lines are used where only 1 would be necessary.


I'm sure if we release this version to the Market, there will be many user complaints that the new layout changed radically from the old layout and wastes a lot of space (for most users).


Since there doesn't seem to be any way to achieve things through xml only - would there be any way to dynamically adjust the layout through code?


On the other hand, you have been working on this task for a very long time, and it is admittedly a very hard task for which we apparently also don't have a perfect solution.


If aap agrees or does not have any better idea, I'm inclined to mark this task as successfully closed (because you have been trying a couple of things that unfortunately didn't work to everyone's satisfaction), but not commit the patch and leave the corresponding issue open. Let's see if aap has to add something...

Shuhao on December 30 2011 22:25 UTC Dynamic changes

I see the problem now..


Dynamic changes are indeed a problem in itself as well. There are a lot of scenerios to consider and it would definitely take a lot of code to get correctly.


And yeah.. I don't think I can come up with a better solution at the moment.. Each item in the list is rather.. crowded.

Peli on December 31 2011 17:11 UTC Ok

I'd say you tried differerent layouts as is evident by the various patches you provided, but none worked satisfactorily. It turns out that considerably more work is required to fix this issue satisfactorily.


Let's close this issue for now so that we don't have to drag this task over to next year. Please pick a simpler task for 2012 :-)

Peli on December 31 2011 17:12 UTC Task Closed

Congratulations, this task has been completed successfully.