-
-
Notifications
You must be signed in to change notification settings - Fork 508
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
Enhancing UX for adding content #2213
Conversation
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if either of these two conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersWhen your PR has been assigned reviewers contact them to get your code reviewed and approved via:
Reviewing Your CodeYour reviewer(s) will have the following roles:
Other🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2213 +/- ##
===========================================
+ Coverage 79.85% 80.29% +0.44%
===========================================
Files 152 149 -3
Lines 7456 6964 -492
===========================================
- Hits 5954 5592 -362
+ Misses 1502 1372 -130 ☔ View full report in Codecov by Sentry. |
@Ayush0Chaudhary @noman2002 I have written tests for all the lines I have added or edited. |
lib/constants/routing_constants.dart
Outdated
@@ -99,4 +99,7 @@ class Routes { | |||
|
|||
/// static variable to access pinnedpostscreen. | |||
static const String pinnedPostScreen = '/pinnedpostscreen'; | |||
|
|||
/// static variable. |
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.
Write a better doc.
lib/router.dart
Outdated
case Routes.addPostScreen: | ||
return MaterialPageRoute( | ||
builder: (context) => const AddPost( | ||
key: Key('addpostscreen'), |
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.
Use camelCase naming
@Dante291 since it is a UI change, it'll help if you can please add before and after screen records. |
@Dante291 add code coverge ss too |
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.
Make sure you cover all the lines.
Before it looked like this: After: Added floating action button for making new post and removed its option from BottomNavBar so that we could have only 4 items in navbar when chat functionality will be implemented again for better UX, this also ensures no more than 1 Add/plus sign button are visible in any screen of the app to add content. |
@literalEval I tried using SizeConfig for setting up the size of the icon but it showed me error, Invalid constant value, I also noticed constant sizing is used for setting up the size for Icons throughout the app. Other than this I have resolved all the above asked changes. |
Remove const from the widget. You'll not get the error. |
@noman2002 I didn't notice that, Thanks sir it helped. Please review or if you have any other UI or any changes do lemme know. |
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.
Looks good to me.
* Enhancing UX for adding content * resolving asked changes * fixing constant sizing
What kind of change does this PR introduce?
Enhancing UX for adding content so that no longer add sign button are there in a particular screen of the App.
Issue Number:
Fixes #2191
Did you add tests for your changes?
Yes
Snapshots/Videos:
![Screenshot_1701688091](https://private-user-images.githubusercontent.com/103507835/287678230-189abbeb-fd15-44b7-88b1-b29210be188d.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkyNzk4MTAsIm5iZiI6MTczOTI3OTUxMCwicGF0aCI6Ii8xMDM1MDc4MzUvMjg3Njc4MjMwLTE4OWFiYmViLWZkMTUtNDRiNy04OGIxLWIyOTIxMGJlMTg4ZC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwMjExJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDIxMVQxMzExNTBaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT04OTE4OTgyMWRhZTI3NmUyMGM4ZjQwMmZlYTlmM2VlZTI2NDE4N2FjZTExOTliZTk3MDFkMGY1M2VjZTU2ODU4JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.Cz5deafFUgvwYINSfD904Pktgpc9b3bkmfbICwKBMNA)
Summary
Currently there are more than one add/plus sign button on some screen of the app to add content this affects the user experience of the App. I added Floating Action Button(FAB) on the feed screen of the app so that there wouldn't be more than one add/plus button on any screen of the app.
This FAB navigates us to the add post page screen to make new posts, took reference from X/Twitter app for making new tweets.
Have you read the [contributing guide] (https://github.com/PalisadoesFoundation/talawa/blob/master/CONTRIBUTING.md)?
Yes