SwEnt-Group13 / Unio

The world’s largest campus life platform.
3 stars 1 forks source link

Feature/event list UI - Refactor Event Handling and UI Improvements #51

Closed Aurelien9Code closed 1 week ago

Aurelien9Code commented 1 week ago

According to the feedback of the majority of the group today, this PR will be cut in two (maybe three) to keep it small. Second part is not finished yet and comes tomorrow moring. Please let me know if that is okay for you in this way !

This PR introduces several structural enhancements and UI adjustments to the events feature to improve code organization and align with the Figma design specifications

IMPROVEMENTS for second part of the PR: -> need to make sure the title text (and basically any text here) is not too long, so that i don't destroy the structure/make another component disappear -> find material3 compenents for little lines to be even more like the Figma + find the same font -> support many association logos (create superposition of logos) -> put security check on the data received, to be sure formatting it for the card will not cause errors -> write some tests supporting the multiplication of Event card item and different sizes of screens (which was a problem when i created my first tests on the subject) -> link Event List page to Home navigation bar to see it without preview

Thanks !

Aurelien9Code commented 1 week ago

Instead of replying to all the message for ui, colors, and harcorded texts, i will do it here as it is the same answer : Basically, as two of the team members requested a first version of my work to continue, i pushed a really lighted version of what should be done for this file as explained in my pull request message And thought about doing all the end work in my second PR

-> For all the harcoded strings, i am currently working on another branch to generalize this process to all the files of the project and was thinking of connecting this to my work when pull to main -> For the hardcoded colors, thanks Zafar ! As explained above, i thought to create a new enum of colors based on the Figma one for the all project -> For the UI, same as above, will be improved in second PR

Aurelien9Code commented 1 week ago

Is it okay for everyone if I do this for the current PR :
-> generalize my two format function in formatTimestamp(timestamp:Timestamp, dateFormat: SimpleDateFormat) as you said ; ) -> put helper function into a class -> remove unused variables (good catch)

And this in PR2 : -> handle multiple association logo superposition, then link it to the image loading when implemented -> modify UI to match even more with Figma (good colors used, good fonts, little lines) and maybe have some ideas to enhance it -> Use MaterialTheme.colorScheme colors instead of random colors

And another PR that will be pushed this afternoon : -> generalize all the strings (for both language) and colors so that they are not hardcoded

Aurelien9Code commented 1 week ago

As you may see in this big message, there is a contradiction between the use of MaterialTheme.colorScheme and the creation of an enum with all the colors of the project. The reason is I don't know yet if all the colors of the project are in the colorScheme. If not, I will also create the enum so that every one can pick and add other colors and not hardcode them.

Aurelien9Code commented 1 week ago

Also i forgot to say why two PRs : -> because some of my coworkers need my code -> because i was dangerously reaching +300 lines of code

Otherwise, one PR would have been better i agree (and starting a branch on my branch for my coworkers)

sonarcloud[bot] commented 1 week ago

Quality Gate Failed Quality Gate failed

Failed conditions
65.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud