Skip to content

Details background#5

Open
ibrkhalil wants to merge 17 commits into
developfrom
detailsBackground
Open

Details background#5
ibrkhalil wants to merge 17 commits into
developfrom
detailsBackground

Conversation

@ibrkhalil
Copy link
Copy Markdown
Owner

No description provided.

@ibrkhalil ibrkhalil requested a review from mcherri September 26, 2021 13:23
Comment thread components/itemDetailsScene.xml Outdated
Comment thread components/itemDetailsScreen.brs
Comment thread components/itemDetailsScreen.brs Outdated
Comment thread components/itemDetailsScreen.brs Outdated
Comment thread components/itemDetailsScreen.brs Outdated
Comment thread components/rowItemDataContent.xml Outdated
Comment thread components/rowListScene.xml Outdated
Comment thread components/tasks/createScreen.xml Outdated
Comment thread components/itemDetailsScreen.brs Outdated
Comment thread components/itemDetailsScreen.brs Outdated
Comment thread components/itemDetailsScreen.brs Outdated
Comment thread components/rowItemDataContent.xml Outdated
Copy link
Copy Markdown
Collaborator

@mcherri mcherri left a comment

Choose a reason for hiding this comment

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

Please fix.

Comment thread components/tasks/createScreen.xml Outdated
Copy link
Copy Markdown
Collaborator

@mcherri mcherri left a comment

Choose a reason for hiding this comment

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

More issues to fix.

Comment thread components/rowListScene.brs Outdated
Comment thread components/rowListScene.brs Outdated
Comment thread components/rowListScene.brs Outdated
Comment thread components/tasks/createScreen.brs Outdated
Comment thread components/tasks/createScreen.xml Outdated
@@ -1,12 +1,21 @@
sub init()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The logic in this file is complex. Please simplify it.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I've added some comments to explain it
or did you mean to rewrite it in a simpler way ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I will help you with this.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Thanks so much ..
I'll be waiting

Comment thread .history/components/itemDetailsScreen_20211008143034.brs Outdated
Comment thread .history/components/itemDetailsScreen_20211008143306.brs Outdated
Comment thread .history/components/itemDetailsScreen_20211008143308.brs Outdated
Comment thread .history/components/itemDetailsScreen_20211008143412.brs Outdated
Comment thread .history/components/rowListScene_20211008143034.brs Outdated
Comment thread .history/components/rowListScene_20211008144149.brs Outdated
Comment thread .history/components/rowListScene_20211008144320.brs Outdated
Comment thread .history/components/rowListScene_20211008145431.brs Outdated

' Handling Responsivness
videoMode = createObject("roDeviceInfo")
if videoMode.GetVideoMode() = "720p"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I will simplify this code for you when I have time. There is a better way to do it.

Comment thread components/rowListScene.brs Outdated
Comment on lines +37 to +44
sub handleHeroDetails()
if m.flag
m.flag = false
handleUpdate()
else
handleItemFocusChange()
end if
end sub
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

An example of complexity is this function. It handles two different concerns.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I see what you mean, Mr. Mostafa

@mcherri
Copy link
Copy Markdown
Collaborator

mcherri commented Oct 30, 2021

Let us discuss what I changed and why next Monday.

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