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

Made the posts UI better #2218

Closed
wants to merge 18 commits into from
Closed

Made the posts UI better #2218

wants to merge 18 commits into from

Conversation

ShubhamTiwari55
Copy link
Contributor

What kind of change does this PR introduce?

Enhanced the UI of posts page by making it vertically scrollable.

Issue Number:

Fixes #2196

Did you add tests for your changes?
No

Snapshots/Videos:

WhatsApp Image

If relevant, did you update the documentation?
No

Summary

I used listview to make the post page like a news feed which enhanced its UI and now it is looking better.

Does this PR introduce a breaking change?

No

Copy link

github-actions bot commented Dec 6, 2023

Our Pull Request Approval Process

We have these basic policies to make the approval process smoother for our volunteer team.

Testing Your Code

Please make sure your code passes all tests. Our test code coverage system will fail if either of these two conditions occur:

  1. The overall code coverage drops below the target threshold of the repository
  2. Any file in the pull request has code coverage levels below the repository threshold

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.

Reviewers

When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

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.

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7d74da9) 87.65% compared to head (7b5b05f) 82.34%.

❗ Current head 7b5b05f differs from pull request most recent head 79f5b13. Consider uploading reports for the commit 79f5b13 to get more accurate results

Files Patch % Lines
...views/after_auth_screens/profile/profile_page.dart 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2218      +/-   ##
===========================================
- Coverage    87.65%   82.34%   -5.32%     
===========================================
  Files          155      151       -4     
  Lines         7356     7029     -327     
===========================================
- Hits          6448     5788     -660     
- Misses         908     1241     +333     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@noman2002
Copy link
Member

@ShubhamTiwari55 The asset is not loading in the screenshot, how can we check the post design. please upload a proper image showing before and after.

@ShubhamTiwari55
Copy link
Contributor Author

ShubhamTiwari55 commented Dec 6, 2023

@ShubhamTiwari55 The asset is not loading in the screenshot, how can we check the post design. please upload a proper image showing before and after.

Here is the updated screenshot
Description of the image

@noman2002
Copy link
Member

noman2002 commented Dec 6, 2023

@ShubhamTiwari55 Alse attach a screenshot of before.

@noman2002
Copy link
Member

@ShubhamTiwari55 Instead of just image. Use the same component used on home screen for posts.

@ShubhamTiwari55
Copy link
Contributor Author

@ShubhamTiwari55 Alse attach a screenshot of before.

Previously it was like this -
WhatsApp Image 2023-12-06 at 17 57 13_67eca8da

@ShubhamTiwari55
Copy link
Contributor Author

ShubhamTiwari55 commented Dec 6, 2023

@ShubhamTiwari55 Instead of just image. Use the same component used on home screen for posts.

Okay, doing the required changes

@ShubhamTiwari55
Copy link
Contributor Author

ShubhamTiwari55 commented Dec 6, 2023

@ShubhamTiwari55 Instead of just image. Use the same component used on home screen for posts.

@noman2002 Is it okay now ?
WhatsApp Image 2023-12-06 at 19 05 05_f0c8b043

@noman2002
Copy link
Member

noman2002 commented Dec 7, 2023

@ShubhamTiwari55 Instead of just image. Use the same component used on home screen for posts.

@noman2002 Is it okay now ? WhatsApp Image 2023-12-06 at 19 05 05_f0c8b043

This is not the component used on home screen. It has comments, likes and etc.

@ShubhamTiwari55
Copy link
Contributor Author

ShubhamTiwari55 commented Dec 7, 2023

@ShubhamTiwari55 Instead of just image. Use the same component used on home screen for posts.

@noman2002 Is it okay now ? WhatsApp Image 2023-12-06 at 19 05 05_f0c8b043

This is not the component used on home screen. It has comments, likes and etc.

Is it okay now ??
Image 1
Image 2

@noman2002
Copy link
Member

@ShubhamTiwari55 Instead of just image. Use the same component used on home screen for posts.

@noman2002 Is it okay now ? WhatsApp Image 2023-12-06 at 19 05 05_f0c8b043

This is not the component used on home screen. It has comments, likes and etc.

Is it okay now ?? Image 1 Image 2

Yes, perfect now. Please write the tests for missing lines.

Ayush0Chaudhary
Ayush0Chaudhary previously approved these changes Dec 8, 2023
@noman2002
Copy link
Member

@ShubhamTiwari55 Merge the latest code into this branch.

@ShubhamTiwari55
Copy link
Contributor Author

ShubhamTiwari55 commented Dec 13, 2023

@ShubhamTiwari55 Merge the latest code into this branch.

@noman2002 I have merged it already. Kindly merge this PR as all changes have been done.

],
),
),
IndividualPostView(post: samplePost),
Copy link
Member

Choose a reason for hiding this comment

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

This is still not covered. Connect with @Ayush0Chaudhary or @Dante291, anyone will help you out.

@ShubhamTiwari55
Copy link
Contributor Author

@Ayush0Chaudhary Can you please help me out in test part ?

@Dante291
Copy link
Contributor

Dante291 commented Dec 16, 2023

Hello @ShubhamTiwari55 hope you're doing well, ok so I checked out your commits and saw that in your test you've commented out the test for IndividualPostView(post: samplePost), this line, so I am considering you're facing difficulty in writing test for that widget.

Testing for UI is basically different than Unit testing as you need to check whether the required widgets appear on the screen and is performing task properly or not, in your case all you had to do is check whether the Individual post widget is present in profile page view.

You need to create the demo or mock Individual post widget in your test file and then create a test to check whether it shows up at the right section of the UI, that's it.

If you still get stuck hit me up on slack or ask your query in Talawa-mobile channel.

@ShubhamTiwari55
Copy link
Contributor Author

ShubhamTiwari55 commented Dec 17, 2023

Hello @ShubhamTiwari55 hope you're doing well, ok so I checked out your commits and saw that in your test you've commented out the test for IndividualPostView(post: samplePost), this line, so I am considering you're facing difficulty in writing test for that widget.

Testing for UI is basically different than Unit testing as you need to check whether the required widgets appear on the screen and is performing task properly or not, in your case all you had to do is check whether the Individual post widget is present in profile page view.

You need to create the demo or mock Individual post widget in your test file and then create a test to check whether it shows up at the right section of the UI, that's it.

If you still get stuck hit me up on slack or ask your query in Talawa-mobile channel.

I have added the tests for IndividualPostView(post: samplePost), also but still the code coverage is not 100%. Here is the screenshot showing all tests passes for that test code also.
Screenshot 2023-12-17 113603

Can you suggest me what should I add more in order to reach the coverage to 100%

@Dante291
Copy link
Contributor

Dante291 commented Dec 17, 2023

 testWidgets("Check if the IndividualPostView shows up",
        (WidgetTester tester) async {
      await tester.runAsync(() async {
        ///making a sample data
        /// also wrote all the detail
        /// used in the individual post
        final commentJson1 = {
          'creator': {
            '_id': '123',
            'firstName': 'Ayush',
            'lastName': 'Chaudhary',
            'email': '[email protected]',
          },
          'createdAt': '123456',
          'text': 'test text',
          'post': 'test post',
          'likeCount': 'test count',
        };
        final commentJson2 = {
          'creator': {
            '_id': '123',
            'firstName': 'John',
            'lastName': 'Doe',
            'email': '[email protected]',
          },
          'createdAt': '123456',
          'text': 'test text',
          'post': 'test post',
          'likeCount': 'test count',
        };

        final commentsJson = [commentJson1, commentJson2];

        ///returning a service response through mocking,
        ///this method was called in
        ///comment view model and was returning null
        ///until I made a mock response.
        when(commentsService.getCommentsForPost('PostID'))
            .thenAnswer((realInvocation) async {
          return commentsJson;
        });

        /// using the mock post made before me
        final Post post = getPostMockModel();

        ///returning mock reponse of post model when
        ///.likedby is called on the model
        when(post.likedBy).thenReturn([LikedBy(sId: "xzy1")]);

        await tester.pumpWidget(createIndividualPostViewWidget(post1: post));
        await tester.pump();

        /// checking if the individual post
        /// screen pops up
        expect(find.byType(Scaffold), findsNWidgets(2));

        ///finding the text field via its ID
        final textfield = find.byKey(const Key('indi_post_tf_key'));

        /// check if the textfield shows up
        expect(textfield, findsOneWidget);

        ///finding the text field via its type
        final textbtn = find.byType(TextButton);

        ///check if btn exist
        expect(textbtn, findsOneWidget);

        ///checking if the onPressed of
        ///text button is working when tapped
        await tester.tap(textbtn);
        await tester.pump();

        ///tapping the text field
        ///to check if it shows up
        await tester.tap(textfield);
        await tester.pump();

        /// Checking if the submit button is working,
        /// when keyboard send action is called
        await tester.showKeyboard(textfield);

        /// keyboard action can be
        /// simulated using the
        /// testTextInput method
        await tester.testTextInput.receiveAction(TextInputAction.send);
        await tester.pump();
      });

@ShubhamTiwari55 you're separately checking Individual post by directly rendering it by doing this await tester.pumpWidget(createIndividualPostViewWidget(post1: post)); but you need to render 'ProfilePage' widget. This is crucial because 'samplePost' is used within 'ProfilePage'. once you render 'ProfilePage' by doing this await tester.pumpWidget(ProfilePage(/* Pass necessary parameters or mocks */)); in your test then you need to assert that IndividualPostView is present. you could do that by looking for specific widgets or text that would only be rendered if IndividualPostView is correctly receiving samplePost.

@Azad99-9
Copy link
Contributor

Azad99-9 commented Dec 19, 2023

@ShubhamTiwari55 for all the UI changes you have made in profile_page.dart please replicate them in demo_profile_page.dart also.

@Dante291
Copy link
Contributor

@Azad99-9 I don't think that it would be required because @ShubhamTiwari55 is working on showing some sample post in the profile_page view but since demo_profile_page.dart file contains the view only for tutorial, there is no need to show any type of post in it? demo_profile_page_.dart file is only for the tour of the app before user join in any organization or log in, since no user is logged in then there is no need for any sample post to be displayed.

@@ -28,6 +33,40 @@ class ProfilePage extends StatelessWidget {
///
final MainScreenViewModel? homeModel;

/// a_line_ending_with_end_punctuation.
Copy link
Member

Choose a reason for hiding this comment

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

Write a meaningful documentation here. Moreover, any mock data should be outside the component. If necessary, create a new file for this.

///
/// **returns**:
/// None
void main() {
Copy link
Member

Choose a reason for hiding this comment

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

Documentation for main function is not needed. Please remove.

class ProfilePage extends StatelessWidget {
const ProfilePage({
ProfilePage({
Copy link
Member

Choose a reason for hiding this comment

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

Remove dummy data as suggested and mark this class as const again. Having const classes improves performance; we should not remove them unnecessarily.

@Azad99-9
Copy link
Contributor

@Dante291 yeah but already there are these hard coded images in demo_profile_page.dart which are being shown in previous format 'tile format' this may cause UI inconsistentcy.

or else @ShubhamTiwari55 just please remove these hard coded images from demo_profile_page.dart. This way we can maintain Consistent UI.

@ShubhamTiwari55
Copy link
Contributor Author

Dear @palisadoes,

I hope this message finds you well. I wanted to inform you that due to my ongoing college end semester exams, I have been unable to dedicate the necessary time and focus to the issue associated with the Pull Request #2218

In light of this, and after discussing with @Dante291 on Slack, we believe that they have a better solution for the issue at hand. Therefore, I am closing the PR and unassigning myself from this particular issue. I have full confidence that @Dante291 will effectively handle and contribute to the resolution of this matter.

I sincerely apologize for any inconvenience caused, and I appreciate the understanding and support from the team. As my exams are concluding, I look forward to resuming regular contributions to the organization.

Once again, thank you to everyone for your valuable reviews and assistance.

Best regards,
Shubham Tiwari

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.

Profile page posts don't have an intuitive interface
6 participants