Skip to content

SVA-444 update filters to new dropdown designs#219

Open
mirkiy wants to merge 16 commits into
developfrom
SVA-444-update-filters-to-new-dropdown-designs
Open

SVA-444 update filters to new dropdown designs#219
mirkiy wants to merge 16 commits into
developfrom
SVA-444-update-filters-to-new-dropdown-designs

Conversation

@mirkiy
Copy link
Copy Markdown
Contributor

@mirkiy mirkiy commented Nov 21, 2024

Jira ticket: SVA-444- Roles List - update filters to new drop down designs (curently called projects list)

Copy link
Copy Markdown
Contributor

@dkaschl dkaschl left a comment

Choose a reason for hiding this comment

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

Give the rather unclear scope and explanation of the ticket mixed with a design where I'm not sure which version we really need to implement, it's a pretty complex and difficult task! As already mentioned it would be important to clarify the exact scope of this ticket and also address what the UI should really be in order to know what exactly what we need to do :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please don't commit this file - probably some temp file?

Comment thread src/NativeBase/Components/SearchIconHighlighted.tsx Outdated
import SkeletonLoading from '../Components/SkeletonLoading'
import FreeSearchBar from '../Components/FreeSearchBar'
import MaterialIcons from 'react-native-vector-icons/MaterialIcons'
import TagButtons from '../Components/TagButtons'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove unused imports (do you see them as unused in your IDE? otherwise activating linting would help and there is afaik also the possibility to auto-clean imports on save)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

was adjusting this page part of this ticket scope?

return (
<HStack space={2} alignItems="center">
{tags.map(tag => (
<Pressable
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd also extract a single tagbutton into it's own component. This component here (TagButtons) could be then just using those tagbutton components. Naming wise we could also name HTagButtons (H for horinzontal) but as we do have only one version currently I think FilterTagButtons would be a good name as it says that it's about tag buttons used for filtering.

</HStack>

{/* Display filtered roles based on the search */}
<SearchResultsView>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not exactly sure how the search result should be implemented, but when it comes to results I suppose it should be the same style like the other lists and not in this section here. Also searching after every keychange is performancewise a bit costly (and makes the system also slow) so I'd advise here to use something like a debouncing pattern.


<VStack>
<ScrollView horizontal showsHorizontalScrollIndicator={false}>
<ProjectSearchContainer />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should rethink about the structure and/or the naming of the components. In this component we put together the freesearchbar with it's extra icon on the right "unwrapped" but then wrapp the tagbuttons with it's choicelists in "ProjectSearchContainer" which is actually not the correct naming as it's actually filtering.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've added comments on how this component is used in ListContainer.tsx. I think we should rename it and maybe restructure it too.

Comment thread src/Navigators/Main.tsx Outdated
} from '@react-navigation/bottom-tabs'
import { useRoute } from '@react-navigation/native'
import { ListContainer } from '@/NativeBase/Containers'
import { ListContainer, ProjectSearchContainer } from '@/NativeBase/Containers'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove unused imports

import { ProjectsState } from '@/Store/Projects'
import { searchByArray, fuzzySearchByArray } from '@/Utils/Search'

enum Tab {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i would have kept the enum, as we would use only the enums throughout this code and if we want to change a name to another one we'd only change it once here.

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.

2 participants