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

Date ordering preference for recipes #1704

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Date ordering preference for recipes #1704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d49b634
Adding some UI elements to play with date-ordering functionality
fredex42 Oct 3, 2024
c56d066
re-lint added bits
fredex42 Oct 18, 2024
7d994ea
tidy up the UI; add a "forcefulness" control that allows the user to …
fredex42 Oct 18, 2024
827cc00
Allow a JSX element into the generic FeedItem card. Nicer time displa…
fredex42 Oct 18, 2024
5db0891
Fix typo, don't overwrite recipes feed when loading non-search data
fredex42 Oct 18, 2024
45377a0
hide pinboard buttons for recipes
fredex42 Oct 18, 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
2 changes: 1 addition & 1 deletion fronts-client/src/bundles/recipesBundle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const fetchRecipesById =
const recipes = await liveRecipes.recipesById(idList);
dispatch(
actions.fetchSuccess(recipes, {
order: recipes.map((_) => _.id),
ignoreOrder: true,
}),
);
} catch (e) {
Expand Down
21 changes: 16 additions & 5 deletions fronts-client/src/components/feed/FeedItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const Byline = styled.h2`
font-weight: bold;
`;

const Title = styled.h2`
export const Title = styled.h2`
margin: 2px 2px 0 0;
vertical-align: top;
font-family: TS3TextSans;
Expand Down Expand Up @@ -111,6 +111,7 @@ interface FeedItemProps {
id: string;
type: CardTypes;
title: string;
bodyContent?: JSX.Element;
liveUrl?: string;
metaContent?: JSX.Element;
scheduledPublicationDate?: string;
Expand All @@ -125,6 +126,7 @@ interface FeedItemProps {
) => void;
shouldObscureFeed?: boolean;
byline?: string;
noPinboard?: boolean;
}

export class FeedItem extends React.Component<FeedItemProps, {}> {
Expand All @@ -138,6 +140,7 @@ export class FeedItem extends React.Component<FeedItemProps, {}> {
id,
type,
title,
bodyContent,
liveUrl,
isLive,
metaContent,
Expand All @@ -149,6 +152,7 @@ export class FeedItem extends React.Component<FeedItemProps, {}> {
hasVideo,
handleDragStart,
byline,
noPinboard,
} = this.props;

const { preview, live, ophan } = getPaths(id);
Expand Down Expand Up @@ -199,10 +203,16 @@ export class FeedItem extends React.Component<FeedItemProps, {}> {
<ShortVerticalPinline />
</MetaContainer>
<Body>
<Title data-testid="headline">{title}</Title>
{byline ? (
<Byline data-testid="byline">{byline}</Byline>
) : undefined}
{bodyContent ? (
bodyContent
) : (
<>
<Title data-testid="headline">{title}</Title>
{byline ? (
<Byline data-testid="byline">{byline}</Byline>
) : undefined}
</>
)}
</Body>
<ArticleThumbnail
style={{
Expand All @@ -221,6 +231,7 @@ export class FeedItem extends React.Component<FeedItemProps, {}> {
toolTipPosition={'top'}
toolTipAlign={'right'}
urlPath={liveUrl}
noPinboard={noPinboard}
renderButtons={(props) => (
<>
<HoverViewButton hoverText="View" href={href} {...props} />
Expand Down
30 changes: 29 additions & 1 deletion fronts-client/src/components/feed/RecipeFeedItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ import { useDispatch } from 'react-redux';
import { insertCardWithCreate } from 'actions/Cards';
import { selectors as recipeSelectors } from 'bundles/recipesBundle';
import { handleDragStartForCard } from 'util/dragAndDrop';
import { Title as FeedItemTitle } from './FeedItem';
import format from 'date-fns/format';

interface ComponentProps {
id: string;
showTimes: boolean;
}

export const RecipeFeedItem = ({ id }: ComponentProps) => {
export const RecipeFeedItem = ({ id, showTimes }: ComponentProps) => {
const shouldObscureFeed = useSelector<State, boolean>((state) =>
selectFeatureValue(state, 'obscure-feed'),
);
Expand All @@ -33,6 +36,15 @@ export const RecipeFeedItem = ({ id }: ComponentProps) => {
);
}, [recipe]);

const renderTimestamp = (iso: string) => {
try {
const date = new Date(iso);
return format(date, 'HH:mm on do MMM YYYY');
} catch (err) {
console.warn(err);
return iso;
}
};
return (
<FeedItem
type={CardTypesMap.RECIPE}
Expand All @@ -45,6 +57,22 @@ export const RecipeFeedItem = ({ id }: ComponentProps) => {
handleDragStart={handleDragStartForCard(CardTypesMap.RECIPE, recipe)}
onAddToClipboard={onAddToClipboard}
shouldObscureFeed={shouldObscureFeed}
noPinboard={true}
bodyContent={
<>
<FeedItemTitle>{recipe.title}</FeedItemTitle>
emdash-ie marked this conversation as resolved.
Show resolved Hide resolved
{recipe?.lastModifiedDate && showTimes ? (
<ContentExtra>
Modified {renderTimestamp(recipe.lastModifiedDate)}
</ContentExtra>
) : undefined}
{recipe?.publishedDate && showTimes ? (
<ContentExtra>
Published {renderTimestamp(recipe.publishedDate)}
</ContentExtra>
) : undefined}
</>
}
metaContent={
<>
<ContentInfo>Recipe</ContentInfo>
Expand Down
136 changes: 130 additions & 6 deletions fronts-client/src/components/feed/RecipeSearchContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import ClipboardHeader from 'components/ClipboardHeader';
import TextInput from 'components/inputs/TextInput';
import { styled } from 'constants/theme';
import React, { useEffect, useState, useCallback } from 'react';
import React, { useCallback, useEffect, useState } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import {
fetchRecipes,
Expand All @@ -18,9 +18,11 @@ import Pagination from './Pagination';
import ScrollContainer from '../ScrollContainer';
import {
ChefSearchParams,
DateParamField,
RecipeSearchParams,
} from '../../services/recipeQuery';
import debounce from 'lodash/debounce';
import ButtonDefault from '../inputs/ButtonDefault';

const InputContainer = styled.div`
margin-bottom: 10px;
Expand All @@ -43,6 +45,9 @@ const PaginationContainer = styled.div`
const TopOptions = styled.div`
display: flex;
flex-direction: row;
justify-content: space-between;
margin-right: 1em;
margin-bottom: 1em;
`;

const FeedsContainerWrapper = styled.div`
Expand All @@ -61,6 +66,12 @@ enum FeedType {
export const RecipeSearchContainer = ({ rightHandContainer }: Props) => {
const [selectedOption, setSelectedOption] = useState(FeedType.recipes);
const [searchText, setSearchText] = useState('');

const [showAdvancedRecipes, setShowAdvancedRecipes] = useState(false);
const [dateField, setDateField] = useState<DateParamField>(undefined);
const [orderingForce, setOrderingForce] = useState<string>('default');
const [forceDates, setForceDates] = useState(false);

const dispatch: Dispatch = useDispatch();
const searchForChefs = useCallback(
(params: ChefSearchParams) => {
Expand All @@ -84,20 +95,37 @@ export const RecipeSearchContainer = ({ rightHandContainer }: Props) => {

const [page, setPage] = useState(1);

/*const debouncedRunSearch = debounce(() => runSearch(page), 750); TODO need to check if needed for chef-search? if yes then how to improve implementing it*/

useEffect(() => {
const dbf = debounce(() => runSearch(page), 750);
dbf();
return () => dbf.cancel();
}, [selectedOption, searchText, page]);
}, [selectedOption, searchText, page, dateField, orderingForce]);

const chefsPagination: IPagination | null = useSelector((state: State) =>
chefSelectors.selectPagination(state),
);

const hasPages = (chefsPagination?.totalPages ?? 0) > 1;

const getUpdateConfig = () => {
switch (orderingForce) {
case 'default':
return undefined;
Comment on lines +112 to +113
Copy link
Contributor

Choose a reason for hiding this comment

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

To be more explicit about how unknown values are handled, could we add a default case here?

Suggested change
case 'default':
return undefined;
case 'default':
default:
return undefined;

case 'gentle':
return {
decay: 0.95,
dropoffScaleDays: 90,
offsetDays: 7,
};
case 'forceful':
return {
decay: 0.7,
dropoffScaleDays: 180,
offsetDays: 14,
};
fredex42 marked this conversation as resolved.
Show resolved Hide resolved
}
};

const runSearch = useCallback(
(page: number = 1) => {
switch (selectedOption) {
Expand All @@ -109,17 +137,25 @@ export const RecipeSearchContainer = ({ rightHandContainer }: Props) => {
case FeedType.recipes:
searchForRecipes({
queryText: searchText,
uprateByDate: dateField,
uprateConfig: getUpdateConfig(),
});
break;
}
},
[selectedOption, searchText, page],
[selectedOption, searchText, page, dateField, orderingForce],
);

const renderTheFeed = () => {
switch (selectedOption) {
case FeedType.recipes:
return recipeSearchIds.map((id) => <RecipeFeedItem key={id} id={id} />);
return recipeSearchIds.map((id) => (
<RecipeFeedItem
key={id}
id={id}
showTimes={forceDates || !!dateField}
/>
));
case FeedType.chefs:
//Fixing https://the-guardian.sentry.io/issues/5820707430/?project=35467&referrer=issue-stream&statsPeriod=90d&stream_index=0&utc=true
//It seems that some null values got into the `chefSearchIds` list
Expand All @@ -144,11 +180,99 @@ export const RecipeSearchContainer = ({ rightHandContainer }: Props) => {
setPage(1);
setSearchText(event.target.value);
}}
onClick={() => setShowAdvancedRecipes(true)}
value={searchText}
/>
</TextInputContainer>
<ClipboardHeader />
</InputContainer>

{showAdvancedRecipes && selectedOption === FeedType.recipes ? (
<>
<TopOptions>
<div>
<label
htmlFor="dateSelector"
style={{ color: searchText == '' ? 'gray' : 'inherit' }}
>
Ordering priority
</label>
</div>
<div>
<select
id="dateSelector"
value={dateField}
disabled={searchText == ''}
onChange={(evt) => {
console.log(evt.target);
setDateField(
evt.target.value === 'Relevance'
? undefined
: (evt.target.value as DateParamField),
);
}}
>
<option value={'Relevance'}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Using an uppercase R here feels inconsistent. I suppose it’s signalling that this option isn’t a field name? I think I’d still prefer lowercase.

Most relevant, regardless of time
</option>
<option value={'publishedDate'}>Most recently published</option>
<option value={'firstPublishedDate'}>
Most recent first publication
</option>
<option value={'lastModifiedDate'}>
Most recently modified
</option>
</select>
</div>
</TopOptions>
<TopOptions>
<div>
<label
htmlFor="orderingForce"
style={{ color: !dateField ? 'gray' : 'inherit' }}
>
Ordering preference
</label>
</div>
<div>
<select
id="orderingForce"
value={orderingForce}
disabled={!dateField}
onChange={(evt) => setOrderingForce(evt.target.value)}
>
<option value={'default'}>Default</option>
<option value={'gentle'}>Prefer relevance</option>
<option value={'forceful'}>Prefer date</option>
</select>
</div>
</TopOptions>
<TopOptions>
<div>
<label htmlFor="forceDateDisplay">Always show dates</label>
</div>
<div>
<input
type="checkbox"
checked={forceDates}
onChange={(evt) => setForceDates(evt.target.checked)}
/>
</div>
</TopOptions>
<TopOptions
style={{
paddingBottom: '0.5em',
borderBottom: '1px solid gray',
marginBottom: '0.5em',
}}
>
<ButtonDefault onClick={() => setShowAdvancedRecipes(false)}>
Close
</ButtonDefault>
</TopOptions>
</>
) : undefined}

<TopOptions>
<RadioGroup>
<RadioButton
Expand Down
4 changes: 3 additions & 1 deletion fronts-client/src/components/inputs/HoverActionButtonWrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ interface WrapperProps {
toolTipAlign: 'left' | 'center' | 'right';
urlPath: string | undefined;
renderButtons: (renderProps: ButtonProps) => JSX.Element;
noPinboard?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling this noPinboard feels like a bit of a double negative: noPinboard={true} takes me a second to think about. Could we make it showPinboard or enablePinboard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know what you mean.... the reason I did it like this is because the pinboard changes are nothing to do with me so it felt better to keep the "default" behaviour to show pinboard (as I don't know what else that may affect) and set the override to hide it rather than vice-versa

Copy link
Contributor

Choose a reason for hiding this comment

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

You could have the prop default to true if not specified, no?

}

export const HoverActionsButtonWrapper = ({
Expand All @@ -53,6 +54,7 @@ export const HoverActionsButtonWrapper = ({
size,
urlPath,
renderButtons,
noPinboard,
}: WrapperProps) => {
const [toolTipText, setToolTipText] = useState<string | undefined>(undefined);

Expand All @@ -79,7 +81,7 @@ export const HoverActionsButtonWrapper = ({
hideToolTip,
size,
})}
{urlPath && (
{urlPath && !noPinboard && (
// the below tag is empty and meaningless to the fronts tool itself, but serves as a handle for
// Pinboard to attach itself via, identified/distinguished by the urlPath data attribute
// @ts-ignore
Expand Down
Loading