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

Jump to bottom

Add ui to switch between recipes and chefs feeds #1588

Merged
merged 3 commits into from
May 31, 2024
Merged

Add ui to switch between recipes and chefs feeds #1588

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
54f76b1
Add radio buttons and hook to handle state and display search accordi…
Divs-B May 24, 2024
fae45f6
Add margin on top for seperation betwen radio buttons and results
Divs-B May 24, 2024
91c3256
Use enum for recipe and chef's feed type and using switch case for re…
Divs-B May 28, 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
78 changes: 49 additions & 29 deletions fronts-client/src/components/feed/RecipeSearchContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import ClipboardHeader from 'components/ClipboardHeader';
import TextInput from 'components/inputs/TextInput';
import ShortVerticalPinline from 'components/layout/ShortVerticalPinline';
import { styled } from 'constants/theme';
import React from 'react';
import React, { useState } from 'react';
import { connect } from 'react-redux';
import { selectors as recipeSelectors } from 'bundles/recipesBundle';
import { selectors as chefSelectors } from 'bundles/chefsBundle';
Expand All @@ -13,6 +13,7 @@ import { SearchResultsHeadingContainer } from './SearchResultsHeadingContainer';
import { SearchTitle } from './SearchTitle';
import { RecipeFeedItem } from './RecipeFeedItem';
import { ChefFeedItem } from './ChefFeedItem';
import { RadioButton, RadioGroup } from '../inputs/RadioButtons';

const InputContainer = styled.div`
margin-bottom: 10px;
Expand All @@ -38,34 +39,53 @@ const FeastSearchContainerComponent = ({
rightHandContainer,
recipes,
chefs,
}: Props) => (
<React.Fragment>
<InputContainer>
<TextInputContainer>
<TextInput placeholder="Search recipes" displaySearchIcon />
</TextInputContainer>
<ClipboardHeader />
</InputContainer>
<FixedContentContainer>
<SearchResultsHeadingContainer>
<SearchTitle>
{'Results'}
<ShortVerticalPinline />
</SearchTitle>
</SearchResultsHeadingContainer>
{Object.values(recipes).map((recipe) => (
<RecipeFeedItem key={recipe.id} recipe={recipe} />
))}
{Object.values(chefs).map((chef) => (
<ChefFeedItem
key={chef.id}
chef={chef}
/> /*TODO: as of now, added rendering of chefs just after the recipes. Need
to change when "search-chefs" action gets finalised*/
))}
</FixedContentContainer>
</React.Fragment>
);
}: Props) => {
const [selectedOption, setSelectedOption] = useState<string>('recipeFeed');
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny point - I've found that if you specify a default value like this, Typescript can infer the type and you don't have to specify it directly with <string>

Copy link
Contributor

Choose a reason for hiding this comment

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

My preference for this is to use numbers, defined as enums (https://www.typescriptlang.org/docs/handbook/enums.html); reason is that it's quicker for the machine to compare and process numbers than strings. However these days I guess that the performance difference is negligible though I would encourage the use of enums or constants over "magic value" strings because if you put a typo in an enum value the compiler complains wheras if you put a typo in a "magic value" string the app breaks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes more sense. updated with enum. thanks @fredex42

const handleOptionChange = (optionName: string) => {
setSelectedOption(optionName);
};
return (
<React.Fragment>
<InputContainer>
<TextInputContainer>
<TextInput placeholder="Search recipes" displaySearchIcon />
</TextInputContainer>
<ClipboardHeader />
</InputContainer>
<RadioGroup>
<RadioButton
checked={selectedOption === 'recipeFeed'}
Copy link
Contributor

Choose a reason for hiding this comment

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

See my point above about defining 'recipeFeed' as an enum value

onChange={() => handleOptionChange('recipeFeed')}
label="Recipes"
inline
name="recipeFeed"
/>
<RadioButton
checked={selectedOption === 'chefFeed'}
onChange={() => handleOptionChange('chefFeed')}
label="Chefs"
inline
name="chefFeed"
/>
</RadioGroup>
<FixedContentContainer>
<SearchResultsHeadingContainer>
<SearchTitle>
{'Results'}
<ShortVerticalPinline />
</SearchTitle>
</SearchResultsHeadingContainer>
{selectedOption === 'recipeFeed'
? Object.values(recipes).map((recipe) => (
<RecipeFeedItem key={recipe.id} recipe={recipe} />
))
: Object.values(chefs).map((chef) => (
<ChefFeedItem key={chef.id} chef={chef} />
))}
</FixedContentContainer>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this is fine for the time being, but the ternary syntax doesn't extend very nicely if we want to put a third option in. To be honest my preference here would be to use a second functional component in the file which contains a switch block on the selectedOption value (passed over as a param) but I wouldn't bother making that change now unless we know that we are going to need a third case 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah changed to switch case, thanks. This will help in scalability.

</React.Fragment>
);
};

const mapStateToProps = (state: State) => ({
recipes: recipeSelectors.selectAll(state),
Expand Down
1 change: 1 addition & 0 deletions fronts-client/src/components/feed/SearchResultsHeadingContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export const SearchResultsHeadingContainer = styled.div`
border-top: 1px solid ${theme.colors.greyVeryLight};
align-items: baseline;
display: flex;
margin-top: 10px;
margin-bottom: 10px;
flex-direction: row;
`;
Loading