echo-swent / echo

4 stars 0 forks source link

Feature/association main screen #294

Closed ntissieres closed 5 months ago

ntissieres commented 6 months ago

The first thing a user sees is the active events of each association she/he follows. Reuse the ListDrawer. On top of this page, we have two buttons (or better MD component) : one with "Followed Associations" which leads to a list of followed associations by a user and one with "My Associations" which leads to a list of associations to which the user belongs. At the bottom right, there is a search button (like the one in HomeScreen) which allows the user to search an association.

ntissieres commented 6 months ago

There are a lot of comments about the UI, which is a bit disappointing because I specifically made a mockup last Friday to avoid that. I asked for it to be reviewed, but got no response, so I started the implementation on Sunday. Anyway:

  1. The problem isn't with this PR, but it's a good point you make, I'll have to adapt it in ListDrawer.
  2. Once again, the problem doesn't come from this PR
  3. That's right, I thought the association names were all small enough to fit in the box. I'm going to change that and let the width of the box adapt.
  4. I thought about doing a special design to make it clear that it's clickable but with the amount of stuff I had to code, I just don't have the time to do it. But I can put the text in a button so that people understand even if it's ugly
  5. Similarly, a better design would make it easier to understand. For now, I can add a title.

About the page for filtering events by association, it's not possible to do that in the HomeScreen. This section of the application is there to highlight associations and their events, which is why I think it's still necessary. About follow/unfollow on the list, this has already been discussed with Jonas here : #251. I'm sorry for the slightly bitter response, but a lot of these comments could have been avoided if someone had told me before I spent 24 hours coding everything to do with associations. But that's OK, thanks anyway for your comments, which are still relevant.

unglazedstamp commented 5 months ago

Sorry to come with problems from other parts of the code, I haven't found them before because I didn't review the screen with the list of events earlier. I will open a separate issue to address them. 1.2. see issue #300

  1. A quick solution could be to use TextButton (https://developer.android.com/develop/ui/compose/components/button#text_button), it would just change the style and color of the text so that it's clear that it's not just text.
  2. I believe the design isn't bad but I found the content a bit surprising given the screen name, maybe we can discuss this today at the meeting to find a better name.

Now that I understand more what's the purpose of the screen, I see why you removed the old ones with the associations lists. I apologize for not giving an earlier feedback for this screen on Figma. I think your design is ok, what triggered my attention was that I though some features were removed from the app.

sonarcloud[bot] commented 5 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
80.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud