Fix an issue (hard)
OpenIntents
Status: Closed
Time to complete: 336 hrs
Mentors: Peli, Friedger Müffke, Randy McEoin, aap, Manuel R. Ciosici
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 |
I would like to work on this task.
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.
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?)
This task has been assigned to Shuhao. You have 336 hours to complete this task, good luck!
I look forward to reviewing your fix. Feel free to discuss your findings and/or approach in the developer forum.
The work on this task is ready to be reviewed.
That's gotta be one of the more difficult task i've done.. After all, UI code is my worst nightmare.
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.
.classpath changed? I tried to make sure it didn't.. and yes it's something Eclipse did. (Someone should really ignore 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...
Yeah. i did all of that.. maybe i missed something this time..
oh well.
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.
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.
I think the way to do it is to wrap everything from quantity to the has_note in a LinearLayout
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.
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)
Please elaborate on the problem with View.GONE. We do use GONE in ShoppingListView.java for some items in the RelativeLayout.
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.
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.
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,
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.
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.
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
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.
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.
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.
By now it's clear that we should have moved this discussion to the developer forum hours ago. Let's continue the discussion there.
The work on this task is ready to be reviewed.
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...
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.
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...
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.
The deadline of the task has been extended with 2 days and 0 hours.
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 has detected that the final deadline has passed and it has reopened the task.
I would like to work on this task.
This task has been assigned to Shuhao. You have 336 hours to complete this task, good luck!
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.
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.
The work on this task is ready to be reviewed.
Finally was able to get it working and I hope this would be it.. I can't find any other problems myself.
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?
If I understood you correctly, I'm pretty sure this is what it does.
Any progress on testing this?
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...
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.
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 :-)
Congratulations, this task has been completed successfully.