Skip to content

Sm menu #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sdemills wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Sm menu #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7afb84d
Initial copy with fonts copied and first pass Menu changes.
sdemills Oct 7, 2011
f279b24
Fonts copied locally and Menu changes in AppsList.
sdemills Oct 7, 2011
f32bbeb
Menu uncreachable code tidied up.
sdemills Oct 7, 2011
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cicada/assets/fonts
View file
Open in desktop

This file was deleted.

Binary file added cicada/assets/fonts/metawatch_16pt_11pxl_proto1.ttf
View file
Open in desktop
Binary file not shown.
Binary file added cicada/assets/fonts/metawatch_8pt_5pxl_CAPS_proto1.ttf
View file
Open in desktop
Binary file not shown.
Binary file added cicada/assets/fonts/metawatch_8pt_7pxl_CAPS_proto1.ttf
View file
Open in desktop
Binary file not shown.
45 changes: 33 additions & 12 deletions cicada/src/org/cicadasong/cicada/AppList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,12 @@ protected void onButtonPress(WatchButton button) {

private void changeSelection(int newIndex) {
selectedIndex = newIndex;
selectedIndex = Math.min(apps.size() - 1, selectedIndex);
selectedIndex = Math.max(0, selectedIndex);
// Wrap around across top or bottom
if (selectedIndex < 0) {
selectedIndex = apps.size() - 1;
} else if (selectedIndex >= apps.size()) {
selectedIndex = 0;
}
invalidate();
}

Expand All @@ -154,20 +158,37 @@ private void launchSelectedApp() {
sendBroadcast(intent);
}

private int lastStartIndex = -1;
private final int listSize = 7;

protected void onDraw(Canvas canvas) {
float y = -paint.ascent();

int startIndex = Math.max(0, selectedIndex - 2);
for (int i = startIndex; (i < apps.size()) && (i < startIndex + 7); i++) {
if (i == selectedIndex) {
paint.setColor(Color.BLACK);
canvas.drawRect(new Rect(0,
(int)(y + paint.ascent()), canvas.getWidth(), (int)(y + paint.descent() + 1)), paint);
}
paint.setColor(i == selectedIndex ? Color.WHITE : Color.BLACK);
canvas.drawText(apps.get(i).appName, LEFT_MARGIN, y, paint);
y += paint.getFontSpacing();
int startIndex = 0;
if (selectedIndex < lastStartIndex) {
startIndex = selectedIndex - (int)(listSize/2); // Put our item in the centre of the list if scrolling upwards
Copy link
Contributor

@joe-trellick joe-trellick Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this part still necessary in light of our previous discussion of paging?

Copy link
Author

@sdemills sdemills Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I didn't notice a response to my point about leaving some overlap?

Copy link
Contributor

@joe-trellick joe-trellick Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't yet fully understand the change to the menu rendering if it doesn't cut down on the phone<->watch traffic. (Like I said, there are probably things that I haven't fully grasped about your change, not having had a chance to try it out yet.) Did the existing layout code break when you added the selection wrap-around?

Copy link
Author

@sdemills sdemills Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - it works fine. If the change of selection is within the items already in view then it originally tried my code to just paint previously selected and newly selected in the appropriate colours but unfortunately the other items them disappeared which was why I commented out that code. With or without that code scrolling off the top of the list brings you nicely back at the bottom of the list, and scrolling off the bottom brings you nicely back to the top. I couldn't fully test getting the newly selected items to appear in the middle of the list because I don't have enough items to exceed the list size and that's what's needed to fully test that. I'd have to add some fictitious items into the set somehow.

Copy link
Contributor

@joe-trellick joe-trellick Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, perhaps we should just take the untested bit out of this change for simplicity's sake, so that we can go ahead and merge in the stuff that we know does work.

} else if (selectedIndex > (lastStartIndex + listSize -1)) {
startIndex = selectedIndex - (int)(listSize/2); // Put our item in the centre of the list if scrolling downwards
}
if ((startIndex + listSize) >= apps.size()) {
startIndex = apps.size() - listSize; // If there would be blank lines then adjust so that doesn't happen if we can
}
if (startIndex < 0) {
startIndex = 0; // If we're off the top then fix it
}

lastStartIndex = startIndex;
Copy link
Contributor

@joe-trellick joe-trellick Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still seems to be changing persistent state in the draw method

Copy link
Author

@sdemills sdemills Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the comment I'm afraid.

Copy link
Contributor

@joe-trellick joe-trellick Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a previous discussion about how changing the persistent state in the onDraw() method could have bad side-effects if onDraw() got called several times between selection changes. It seems like we should be able to make it so that persistent state is only changed when the selection changes, not every time the onDraw() happens.


for (int i = startIndex; (i < apps.size()) && (i < startIndex + 7); i++) {
Copy link
Contributor

@joe-trellick joe-trellick Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this #7 be replaced by listSize?

Copy link
Author

@sdemills sdemills Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it should. I've not done this kind of collaboration before, and since I wanted to see if you approved of what I was doing before committing more time, that number 7 is your original code and didn't need to be changed so I left it untouched. Ultimately we should be computing the listSize, setting that variable, and using it everywhere.

I'm out for the weekend - will have email and internet access, but won't be able to do more on this now until Monday probably - unless we are rained in by bad weather and have a great internet signal in which case I might have the opportunity to have another go before Monday.

Copy link
Contributor

@joe-trellick joe-trellick Oct 7, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, looking forward to continuing when you're back. Enjoy the weekend!

if (i == selectedIndex) {
paint.setColor(Color.BLACK);
canvas.drawRect(new Rect(0,
(int)(y + paint.ascent()), canvas.getWidth(), (int)(y + paint.descent() + 1)), paint);
}
paint.setColor(i == selectedIndex ? Color.WHITE : Color.BLACK);
canvas.drawText(apps.get(i).appName, LEFT_MARGIN, y, paint);
y += paint.getFontSpacing();
}
}

}