Skip to content
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.

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

Feature/731/slim monthview #733

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

Conversation

newhinton
Copy link
Contributor

No description provided.

Copy link
Member

@jspricke jspricke left a comment

Choose a reason for hiding this comment

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

I added some proposals for whitespace fixes (please try to be more consistent to the existing style in future).
I think we need more pagging around the events to improve readability.

src/com/android/calendar/month/MonthWeekEventsView.java Outdated Show resolved Hide resolved
src/com/android/calendar/month/MonthWeekEventsView.java Outdated Show resolved Hide resolved
src/com/android/calendar/month/MonthWeekEventsView.java Outdated Show resolved Hide resolved
src/com/android/calendar/month/MonthWeekEventsView.java Outdated Show resolved Hide resolved
src/com/android/calendar/month/MonthWeekEventsView.java Outdated Show resolved Hide resolved
src/com/android/calendar/month/MonthWeekEventsView.java Outdated Show resolved Hide resolved
src/com/android/calendar/month/MonthWeekEventsView.java Outdated Show resolved Hide resolved
src/com/android/calendar/month/MonthWeekEventsView.java Outdated Show resolved Hide resolved

if(!mEvent.allDay){
r.left+=EVENT_RECT_MARGIN_LEFT;
r.right+=EVENT_RECT_MARGIN_LEFT;
Copy link
Member

Choose a reason for hiding this comment

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

logic here sounds unusual, you add marin_left whereas in line 1559 you substract margin right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is a bit unusual. It seems that the background version behaves differently for those cases, they seem to be differently sized, that is why they have different margins (and why the right margin is added instead of substracted) should i add a comment there why that is in the code?


private static int EVENT_RECT_TEXT_MARGIN_LEFT = 5;
private static int EVENT_RECT_TEXT_MARGIN_RIGHT = 5;
private static int EVENT_RECT_TEXT_MARGIN_TOP= 0;
Copy link
Member

Choose a reason for hiding this comment

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

why 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because if it has an greater offset, the text moves out of the drawingboundaries for the colored chip behind it. I put it in because of completeness, but it could be removed

@newhinton newhinton changed the title WIP: Feature/731/slim monthview Feature/731/slim monthview Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants