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.

Sign up for GitHub

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

[feat] technology filter functionality #85

Merged
merged 9 commits into from
Dec 5, 2024
Merged

[feat] technology filter functionality #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
93f6f67
[feat] filtering using technology filter
ethan-tam33 Nov 21, 2024
9c1dca3
[feat] search over filtered projects
ethan-tam33 Nov 22, 2024
6468f17
[feat] added clear filters function
ethan-tam33 Nov 22, 2024
5f7bd30
added clear function, removed map updates on search
ethan-tam33 Nov 23, 2024
0116ffa
merged with main
ethan-tam33 Nov 24, 2024
eb902a1
[fix] fixed issues
ethan-tam33 Nov 24, 2024
37eee59
[fix] fixed issues
ethan-tam33 Nov 24, 2024
70a37f9
pulled from main
ethan-tam33 Dec 2, 2024
2da013d
[fix] fixed bug
ethan-tam33 Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions api/maps/AddMarkers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export default function AddMarker({
return mapZoom;
};
*/

const clusterer = useMemo(() => {
if (!map) return null;

Expand Down Expand Up @@ -128,18 +129,6 @@ export default function AddMarker({
onClusterClick: clusterHandler,
});

/*setClusterer.addListener('click', function (cluster: Cluster) {
const mapZoom = map.getZoom() ?? 0;
const minZoom = getMinZoom(cluster, mapZoom);

if (mapZoom && mapZoom < minZoom) {
const idleListener = map.addListener('idle', function () {
map.setZoom(minZoom);
idleListener.remove();
});
}
});*/

return setClusterer;
}, [map]);

Expand Down
6 changes: 6 additions & 0 deletions components/Filter/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ interface FilterProps {
selectedFilters: Filters;
filterChangeHandlers: FilterChangeHandlers;
handleButtonClick: (filter: FilterType) => void;
handleFilterButtonClick: () => void;
clearFilters: () => void;
}

export default function Filter({
Expand All @@ -24,6 +26,8 @@ export default function Filter({
selectedFilters,
filterChangeHandlers,
handleButtonClick,
handleFilterButtonClick,
clearFilters,
}: FilterProps) {
return (
<FilterBackgroundStyles isActive={isActive}>
Expand All @@ -36,6 +40,8 @@ export default function Filter({
icon={filter.icon}
label={filter.label}
currFilter={filter}
handleFilterButtonClick={handleFilterButtonClick}
clearFilters={clearFilters}
/>
) : filter.id === 'status' ? (
<StatusDropdown
Expand Down
9 changes: 6 additions & 3 deletions components/FilterBar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,21 @@ import { FilterContainerStyles } from './styles';

export const FilterBar = ({
filters,
onFilterChange,
selectedFilters,
setSelectedFilters,
handleFilterButtonClick,
clearFilters,
}: {
filters: FilterType[];
onFilterChange: (filter: FilterType) => void;
selectedFilters: Filters;
setSelectedFilters: React.Dispatch<React.SetStateAction<Filters>>;
handleFilterButtonClick: () => void;
clearFilters: () => void;
}) => {
const [activeFilter, setActiveFilter] = useState<FilterType | null>(null);

const handleButtonClick = (filter: FilterType) => {
setActiveFilter(activeFilter?.id === filter.id ? null : filter);
onFilterChange(filter);
};

const handleTechnologyChange = (options: string[]) => {
Expand Down Expand Up @@ -52,6 +53,8 @@ export const FilterBar = ({
selectedFilters={selectedFilters}
filterChangeHandlers={filterChangeHandlers}
handleButtonClick={handleButtonClick}
handleFilterButtonClick={handleFilterButtonClick}
clearFilters={clearFilters}
/>
))}
</FilterContainerStyles>
Expand Down
59 changes: 50 additions & 9 deletions components/MapViewScreen/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,29 +56,70 @@ export default function MapViewScreen({
projectSize: { min: 0, max: 0 },
location: [],
});
const [filteredProjectsFromDropdowns, setFilteredProjectsFromDropdowns] =
useState<Project[]>(projects);

const handleFilterChange = (filter: FilterType) => {
console.log(filter);
const [filteredProjectsFromSearch, setFilteredProjectsFromSearch] =
useState<Project[]>(projects);

// clear filters
const clearFilters = () => {
setSelectedFilters({
status: [],
technology: [],
projectSize: { min: 0, max: 0 },
location: [],
});
setFilteredProjects(projects);
setFilteredProjectsFromDropdowns(projects);
};

// show projects based on selected filters
const handleFilterButtonClick = () => {
/* eslint-disable @typescript-eslint/no-unused-vars */
const { status, technology, projectSize, location } = selectedFilters;
Copy link

Choose a reason for hiding this comment

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

I'm unsure how these are used to filter projects. Perhaps we could setup the logic for filtering by all criteria (built in a way that would allow us to add/remove filters). For each project, we'd need to check that each selected feature is satisfied.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh I see, that's a good idea! I made the filters under the impression that they wouldn't need to be changed, but I agree that making the logic more flexible would be better in the long run.

The way filtering works currently is that I have a pre-defined object of possible filters (selectedFilters) that the projects can filter through within the handleFilterButtonClick method. selectedFilters gets updated every time a new filter is applied or un-applied. Is there anything specifically you would want me clarify?

Copy link

Choose a reason for hiding this comment

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

I just wanted to make sure all filters were being used. It seems that the current implementation hardcodes filtering according to the technology criteria. For flexibility purposes, we could iterate over all attributes of the selectedFilters object in a way that is agnostic to what the filters actually are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh I see, I think I'm doing that? The technology variable in line 80 is just a list that holds a bunch of strings (or is empty). Then line 82 filters all the projects by checking if its renewable_energy_technology attribute is within this technology list, without caring about specific values. Is this what you mean about not hardcoding the attributes?

Copy link

Choose a reason for hiding this comment

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

I guess, given that this PR is only for the technology filter, your implementation suffices. I was just curious if there was a way to iterate over the selectedFilters object so we could easily implement status, projectSize, and location filters as well. However doing so in a way where each filter is it's own function/reducer that, when applied to a given row, can immediately determine whether it belongs in the output. Does that make sense? It might not be necessary for this implementation, but it's one possible approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh I see now, you're referring to the general filters. I think we can definitely do that and it makes sense to do so- this probably will involve some mapping of the filters to the project attributes in question. And then the method can just do an agnostic loop over that mapping to filter the projects. Thanks Ronnie! I'll adjust my PR for this.

// add all filtering logic here
const technologyProjects = projects?.filter(project =>
technology.includes(project.renewable_energy_technology),
);
setFilteredProjectsFromDropdowns(technologyProjects);
};

// search within all projects or filtered projects from dropdowns
useEffect(() => {
let filtered: Project[] = [];
filtered =
projects?.filter(project =>
let projectsToSearch: Project[] = [];

if (filteredProjectsFromDropdowns.length > 0) {
projectsToSearch = filteredProjectsFromDropdowns;
} else {
projectsToSearch = projects;
}

const searchedProjects: Project[] =
projectsToSearch?.filter(project =>
Comment on lines +90 to +99
Copy link

Choose a reason for hiding this comment

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

Can filters be applied after a search is made? In that case, would it make sense to search then filter?

I wonder if there's a way to treat "search" similar to a filter criteria. Each filter would then be applied sequentially in the order they're selected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way that I've implemented it allows for filtering after a search is made! I think it does make sense to search then filter? For example, if someone wanted to checkout a project that has a common word "north", "south", etc. and then wanted to filter by dropdowns. This is probably a niche, rare use case but the way that the filtering is implemented allows for search then filter and filter then search to produce the same outputs in the project modal.

Do you mind explaining your idea of sequentially applying filters? I'm not quite sure I understand how the projects would be rendered differently.

Copy link

Choose a reason for hiding this comment

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

Ok it seems that the implementation already works as intended then. I was just curious about the ordering that filters are applied, because it could affect performance.

For example, if you wish to search for "north" and filter by "hydroelectric," it might make sense to filter first then search, because depending on the search implementation, I imagine search would be an asymptotically slower operation (??), and thus better to perform on a smaller subset of projects.

The more I think about it, this is entirely dependent on search logic and how project data is stored, which I don't have much context on. Just something to consider!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with Ronnie. We should filter --> search. Every time the user types a letter to search we are searching based on what the user has typed so far, so I think it would be better to search on a smaller subset. @ethan-tam33 can you adjust your sprint to filter then search?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm are you both suggesting that we prevent any searching until the user clicks on at least one filter?

Regarding the time complexity of searching, I believe the way that we have set it up has filtering for search and dropdown filters to have the same complexity. We're iterating over each project and checking if it includes the search term or is included in the dropdown filters.

Copy link

Choose a reason for hiding this comment

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

I see that search isn't very complex in this case. Although I think the reason I bring this up is based off the main useEffect, where projectsToSearch selects all filtered projects, then the search term is applied to this list. Suppose I have filtered by hydroelectric then searched for "north". I'd now have a list of projects satisfying these criteria. If I additionally applied the operational filter, ideally I'd take the list of already filtered projects, and apply the additional filter there. However, it seems that filters are applied first, separately from search, then the search term is applied after. Do you see what I mean? I'm not sure if that makes sense or whether it's actually needed, but something I noticed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh ok I see what you are saying! I agree that we should do this for filters part of the Status filter. However, I don't think we can do this for a flow like hydroelectric -> search for north -> land-based wind (aka another technology filter). We would need to filter all projects based on the technology filters and those wouldn't exist in the modal prior to selecting land-based wind. Perhaps this is a task that Neha's recent PR can reflect then?

Copy link
Collaborator

Choose a reason for hiding this comment

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

For now, just squash and merge your sprint so that I can check everything works smoothly for this week's sprints. We can sort this out another time.

project.project_name.toLowerCase().includes(searchTerm.toLowerCase()),
) ?? null;
) ?? [];

setFilteredProjects(filtered);
}, [projects, searchTerm, setFilteredProjects]);
setFilteredProjectsFromSearch(searchedProjects);
}, [projects, searchTerm, filteredProjectsFromDropdowns]);

useEffect(() => {
setFilteredProjects(filteredProjectsFromDropdowns);
}, [filteredProjectsFromDropdowns, setFilteredProjects]);

useEffect(() => {
setFilteredProjects(filteredProjectsFromSearch);
}, [filteredProjectsFromSearch, setFilteredProjects]);

return (
<>
<SearchBar searchTerm={searchTerm} setSearchTerm={setSearchTerm} />
<FilterBar
filters={filters}
onFilterChange={handleFilterChange}
selectedFilters={selectedFilters}
setSelectedFilters={setSelectedFilters}
handleFilterButtonClick={handleFilterButtonClick}
clearFilters={clearFilters}
/>
<Map
projects={projects}
Expand Down
20 changes: 15 additions & 5 deletions components/TechnologyDropdown/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
ExitStyles,
FilterContentDiv,
FilterDropdownStyles,
FilterIconStyles,
IconStyles,
} from './styles';

Expand All @@ -35,6 +36,8 @@ interface TechnologyDropdownProps {
icon: React.ReactNode;
label: string;
currFilter: FilterType;
handleFilterButtonClick: () => void;
clearFilters: () => void;
}

export default function TechnologyDropdown({
Expand All @@ -44,6 +47,8 @@ export default function TechnologyDropdown({
icon,
label,
currFilter,
handleFilterButtonClick,
clearFilters,
}: TechnologyDropdownProps) {
const filter = {
categories: [
Expand Down Expand Up @@ -138,12 +143,14 @@ export default function TechnologyDropdown({
return (
<FilterDropdownStyles>
<FilterContentDiv>
<ButtonWithIconStyles onClick={() => handleButtonClick(currFilter)}>
{icon}
<ButtonStyles>
<ButtonWithIconStyles>
<FilterIconStyles onClick={() => handleButtonClick(currFilter)}>
{icon}
</FilterIconStyles>
<ButtonStyles onClick={() => handleButtonClick(currFilter)}>
<FilterHeadingUnused>{label}</FilterHeadingUnused>
</ButtonStyles>
<ExitStyles>
<ExitStyles onClick={clearFilters}>
<ExitIcon />
</ExitStyles>
</ButtonWithIconStyles>
Expand All @@ -169,7 +176,10 @@ export default function TechnologyDropdown({
))}
</div>
))}
<ApplyButtonStyles isActive={isApplyButtonActive}>
<ApplyButtonStyles
isActive={isApplyButtonActive}
onClick={handleFilterButtonClick}
>
<ApplyFiltersText>APPLY</ApplyFiltersText>
</ApplyButtonStyles>
</FilterContentDiv>
Expand Down
6 changes: 6 additions & 0 deletions components/TechnologyDropdown/styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ export const ButtonWithIconStyles = styled.div`
cursor: pointer;
`;

export const FilterIconStyles = styled.div`
display: flex;
align-items: center;
flex-direction: row;
`;
Comment on lines +75 to +79
Copy link

Choose a reason for hiding this comment

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

Nit: Any chance the FilterIconStyles style could extend a button instead of a div? It seems to be using the onClick property in the TechnologyDropdown which is an attribute that typically belongs to buttons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm previously only ButtonWithIconStyles had the onClick property, but I had to manually add it for each of the sub-components in this sprint because of the temporary clearFilters method on the exit button. I don't think this change will be permanent, but I do agree that we should look into making ButtonWithIconStyles extend a button instead of a div.


export const IconStyles = styled.div`
width: '3rem',
height: '3rem',
Expand Down