DashboardView row/col count fixes for iPad; X buttons vanishing fix #1321

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

rf commented Feb 1, 2012

My coworker Aaron Richton reported this a long time ago: http://support.appcelerator.com/tickets/APP-539483/tickets This JIRA ticket was +1'd: http://jira.appcelerator.org/browse/TIMOB-1871

Unfortunately, I was hoping for a fix which would allow me to set the rowHeight and therefore easily support iPad. The ticket was closed with 'fixed' and the code now just sets the number of rows to 3. This solution is unacceptable for iPad.

My change simply sets the default rowCount to 5 and columnCount to 4 on iPad.

The other change (the commented lines around 650) fixes a bug I was experiencing where occasionally the x buttons on the dashboard items would be invisible when entering edit mode.

Contributor

negupta commented Feb 15, 2012

Russ - Did you sign a CLA? If yes, please provide me the sign date and email address that you signed it with.

Contributor

rf commented Feb 16, 2012

Hi,

I signed a CLA on April 27 2011. The email used was rfranknj@nbcs.rutgers.edu.

Contributor

negupta commented Feb 16, 2012

Signed CLA is in place.

@BlainHamon BlainHamon commented on the diff Mar 16, 2012

iphone/Classes/LauncherView.m
@@ -144,15 +155,6 @@ - (NSMutableArray*)pageWithFreeSpace:(NSInteger)pageIndex
return page;
}
-- (NSInteger)rowCount
-{
@BlainHamon

BlainHamon Mar 16, 2012

Contributor

This removal worries me, because it changes the behavior in an unexpected way: it means for a 90 pixel tall area, it no longer falls down to 2 rows of 33 pixels, but insists on keeping 3 rows of 33 pixels. There must be a more elegant solution.

@BlainHamon BlainHamon commented on the diff Mar 16, 2012

iphone/Classes/LauncherView.m
@@ -648,6 +650,9 @@ - (void)endEditing
[UIView setAnimationDelegate:self];
[UIView setAnimationDidStopSelector:@selector(endEditingAnimationDidStop:finished:context:)];
+ /* Removing this as it appears to break the X buttons; they will
@BlainHamon

BlainHamon Mar 16, 2012

Contributor

Something also doesn't smell right here. Disabling the fade functionality instead of fixing the issue will cause a regression among other things.

Contributor

BlainHamon commented Mar 16, 2012

Code currently REJECTED. There are some edge cases and needed fixes that this pull does not currently address, and would cause different behavior which I'm not sure is for the best in all cases.

Contributor

rf commented Mar 16, 2012

Re: the rowCount change, would it be acceptable to hard-code 'acceptable' rowHeight values for iPhone and iPad, then change rowCount to be once again based on the rowHeight?

Contributor

BlainHamon commented Mar 16, 2012

My concern is that there are likely uses of Dashboard that neither of us are considering and would break. For example, what if someone has a popover in their iPad app that uses a dashboard and really wants only 3 rows? Fortunately this is iOS-only, so no issues with parity, and you're right in that we need to have one dependent on the other (and avoid an infinite loop dependancy).

I suppose the best solution is to add customizability. That one can set rowheight and/or rowcount/columncount manually, but the default is still 3 by 3 to avoid default behavioral changes.

Contributor

negupta commented Apr 27, 2012

@russfrank - do you plan to address review comments?

Contributor

rf commented Apr 29, 2012

Sorry, this one slipped through the cracks.

I do not, as I've had to move off of using this code due to compatibility issues. I'll close the request.

rf closed this Apr 29, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment