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

Feature: Add remaining search bars #407

Open
wants to merge 38 commits into
base: develop
Choose a base branch
from

Conversation

julian-wls
Copy link
Contributor

@julian-wls julian-wls commented Feb 12, 2025

Problem Description

Some of the screens in the app do not have search bars yet, see #378.

Changes

This PR introduces a new ArtemisSearchTopAppBar, which has a built in search bar and is used across the different screens of the course. Filtering functionality has been added for the lecture, exercise and browse channels screen. The search bar of the conversations screen and the member screen has been updated to use the new ArtemisSearchTopAppBar.

Important note:
There are two E2E tests that are currently failing, because of some modifications I had to make to the search bar. These modifications are also made in #384 including fixes for these tests. Once #384 is merged the tests will not fail anymore.

Steps for testing

  1. Navigate into a course that has exercises and lectures
  2. Click on the search bar and verify that the transition works correctly
  3. Begin to search and verify everything works as expected

Screenshots

@julian-wls julian-wls self-assigned this Feb 12, 2025
@julian-wls julian-wls changed the title Feature/add remaining searchbars Feature: Add remaining search bars Feb 12, 2025
@julian-wls julian-wls added the ready for review This PR can be reviewed label Feb 12, 2025
@julian-wls julian-wls marked this pull request as ready for review February 12, 2025 10:53
@julian-wls julian-wls requested a review from eylulnc February 12, 2025 10:53
@julian-wls julian-wls linked an issue Feb 12, 2025 that may be closed by this pull request
@FelberMartin
Copy link
Collaborator

Oh boooy, I love the animation when clicking the search bar 😍

One thing I am concerned about is the occupied space by the search bar, especially in tablet mode however. The screen is quite cluttered with Navigation bars.
image

One idea might be make the TopBar collapsable on scroll, and force the topbar to be collapsed in the tablet mode as soon as entering a chat. The collapsed TopBar state could looks like the "old" TopBar, but with a search icon on the top right, to still allow users to switch to the search mode.

I'm really just brain storming at this point though, so let me know what you think!

FelberMartin and others added 2 commits February 13, 2025 09:48
# Conflicts:
#	core/ui/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/core/ui/common/BasicArtemisTextField.kt
#	feature/metis/conversation/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/ConversationChatListScreen.kt
#	feature/metis/manage-conversations/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/manageconversations/ui/conversation/browse_channels/BrowseChannelsScreen.kt
Base automatically changed from improvement/add-elevation-to-top-bar to develop February 13, 2025 10:10
@julian-wls julian-wls removed the ready for review This PR can be reviewed label Feb 14, 2025
@julian-wls julian-wls added the ready for review This PR can be reviewed label Feb 14, 2025
Copy link
Collaborator

@FelberMartin FelberMartin left a comment

Choose a reason for hiding this comment

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

As already said, awesome change, the animations really spice up the app!

A few code comments, most of them are just me being a fan of early return, though :)

Copy link
Contributor

@eylulnc eylulnc left a comment

Choose a reason for hiding this comment

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

Looks awesome. For the code Martin already made lot of comment so I do not have any addition.

@FelberMartin FelberMartin added ready to merge This PR can be merged and removed ready for review This PR can be reviewed labels Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge This PR can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add search bars in remaining places
3 participants