-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: make entire tip resource area clickable #89
Conversation
icon: const Icon(Icons.link), | ||
), | ||
], | ||
body: InkWell( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The body should open and close when clicked, not open the link
@@ -27,7 +27,7 @@ class AllTipsPage extends StatelessWidget { | |||
trailing: const Icon(Icons.arrow_forward), | |||
onTap: () => Navigator.of(context).push( | |||
MaterialPageRoute( | |||
builder: (context) => TipInformationPage(tip), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no reason for change
…nto tip_buttons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You missed a couple of comments
Could you please elaborate? I can't seem to find any comments added to the files in this PR. |
Hey, Thanks |
icon: const Icon(Icons.link), | ||
), | ||
], | ||
body: InkWell( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The body should open and close when clicked, not open the link
I took the liberty of replacing Icon Button with Icon as InkWell makes the entire area clickable and makes Icon Button redundant. I also added Padding to the text to improve the look |
I guess there was a communication gap, I will modify my implementation |
Still working on this, I can't unrequest reviews so don't review it yet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Still working on this, I can't unrequest reviews so don't review it yet"
Here you go
Alright, I'm done |
isExpanded.forEach((key, value) { | ||
isExpanded[key] = false; | ||
}); | ||
isExpanded[isExpanded.keys | ||
.elementAt(panelIndex)] = expanded; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this code, it does not look clean.
fixed issue as mentioned in #86