Skip to content

Conversation

@ibrkhalil
Copy link
Owner

No description provided.

@ibrkhalil ibrkhalil requested a review from mcherri September 26, 2021 13:23
Copy link
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.

Copy link
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.

@@ -1,12 +1,21 @@
sub init()
Copy link
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
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
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
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


' Handling Responsivness
videoMode = createObject("roDeviceInfo")
if videoMode.GetVideoMode() = "720p"
Copy link
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 on lines 37 to 44
sub handleHeroDetails()
if m.flag
m.flag = false
handleUpdate()
else
handleItemFocusChange()
end if
end sub
Copy link
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
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
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.

3 participants