Skip to content

Commit

Permalink
Merge pull request #1625 from Sefaria/bug/sc-20275/chronological-filt…
Browse files Browse the repository at this point in the history
…ering-isn-t-sticky-on-topics

Bug/sc 20275/chronological filtering isn t sticky on topics
  • Loading branch information
nsantacruz committed Sep 18, 2023
2 parents 737e0dc + 4349232 commit ae6899b
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 7 deletions.
1 change: 1 addition & 0 deletions reader/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3059,6 +3059,7 @@ def topic_page(request, topic, test_version=None):
"initialMenu": "topics",
"initialTopic": topic,
"initialTab": urllib.parse.unquote(request.GET.get('tab', 'sources')),
"initialTopicSort": urllib.parse.unquote(request.GET.get('sort', 'Relevance')),
"initialTopicTitle": {
"en": topic_obj.get_primary_title('en'),
"he": topic_obj.get_primary_title('he')
Expand Down
37 changes: 32 additions & 5 deletions static/js/Misc.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -360,15 +360,41 @@ ProfilePic.propTypes = {
};


/**
* Renders a list of data that can be filtered and sorted
* @param filterFunc
* @param sortFunc
* @param renderItem
* @param sortOptions
* @param getData
* @param data
* @param renderEmptyList
* @param renderHeader
* @param renderFooter
* @param showFilterHeader
* @param refreshData
* @param initialFilter
* @param scrollableElement
* @param pageSize
* @param onDisplayedDataChange
* @param initialRenderSize
* @param bottomMargin
* @param containerClass
* @param onSetSort: optional. function that is passed the current sort option when the user changes it. Use this to control sort from outside the component. See `externalSortOption`.
* @param externalSortOption: optional. string that is one of the options in `sortOptions`. Use this to control sort from outside the component. See `onSetSort`.
* @returns {JSX.Element}
* @constructor
*/
const FilterableList = ({
filterFunc, sortFunc, renderItem, sortOptions, getData, data, renderEmptyList,
renderHeader, renderFooter, showFilterHeader, refreshData, initialFilter,
scrollableElement, pageSize, onDisplayedDataChange, initialRenderSize,
bottomMargin, containerClass
bottomMargin, containerClass, onSetSort, externalSortOption,
}) => {
const [filter, setFilter] = useState(initialFilter || '');
const [sortOption, setSortOption] = useState(sortOptions[0]);
const [internalSortOption, setSortOption] = useState(sortOptions[0]);
const [displaySort, setDisplaySort] = useState(false);
const sortOption = externalSortOption || internalSortOption;

// Apply filter and sort to the raw data
const processData = rawData => rawData ? rawData
Expand Down Expand Up @@ -420,10 +446,11 @@ const FilterableList = ({
}, [dataUpToPage]);
}

const onSortChange = newSortOption => {
const setSort = newSortOption => {
if (newSortOption === sortOption) { return; }
setSortOption(newSortOption);
setDisplaySort(false);
onSetSort?.(newSortOption);
};

const oldDesign = typeof showFilterHeader == 'undefined';
Expand Down Expand Up @@ -453,7 +480,7 @@ const FilterableList = ({
isOpen={displaySort}
options={sortOptions.map(option => ({type: option, name: option, heName: Sefaria._(option, "FilterableList")}))}
currOptionSelected={sortOption}
handleClick={onSortChange}
handleClick={setSort}
/>
</DropdownModal>
: null
Expand All @@ -480,7 +507,7 @@ const FilterableList = ({
<span
key={option}
className={classNames({'sans-serif': 1, 'sort-option': 1, noselect: 1, active: sortOption === option})}
onClick={() => onSortChange(option)}
onClick={() => setSort(option)}
>
<InterfaceText context="FilterableList">{option}</InterfaceText>
</span>
Expand Down
4 changes: 4 additions & 0 deletions static/js/ReaderApp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class ReaderApp extends Component {
searchQuery: props.initialQuery,
searchTab: props.initialSearchTab,
tab: props.initialTab,
topicSort: props.initialTopicSort,
textSearchState: new SearchState({
type: 'text',
appliedFilters: props.initialTextSearchFilters,
Expand Down Expand Up @@ -167,6 +168,7 @@ class ReaderApp extends Component {
textHighlights: state.textHighlights || null,
profile: state.profile || null,
tab: state.tab || null,
topicSort: state.topicSort || null,
webPagesFilter: state.webPagesFilter || null,
sideScrollPosition: state.sideScrollPosition || null,
topicTestVersion: state.topicTestVersion || null
Expand Down Expand Up @@ -386,6 +388,7 @@ class ReaderApp extends Component {
(prev.searchQuery != next.searchQuery) ||
(prev.searchTab != next.searchTab) ||
(prev.tab !== next.tab) ||
(prev.topicSort !== next.topicSort) ||
(prev.collectionName !== next.collectionName) ||
(prev.collectionTag !== next.collectionTag) ||
(!prevTextSearchState.isEqual({ other: nextTextSearchState, fields: ["appliedFilters", "field", "sortType"]})) ||
Expand Down Expand Up @@ -479,6 +482,7 @@ class ReaderApp extends Component {
case "topics":
if (state.navigationTopic) {
hist.url = state.topicTestVersion ? `topics/${state.topicTestVersion}/${state.navigationTopic}` : `topics/${state.navigationTopic}`;
hist.url = hist.url + (state.topicSort ? `&sort=${state.topicSort}` : '');
hist.title = `${state.topicTitle[shortLang]} | ${ Sefaria._("Texts & Source Sheets from Torah, Talmud and Sefaria's library of Jewish sources.")}`;
hist.mode = "topic";
} else if (state.navigationTopicCategory) {
Expand Down
5 changes: 5 additions & 0 deletions static/js/ReaderPanel.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,9 @@ class ReaderPanel extends Component {
: false
this.conditionalSetState({tab: tab})
}
onSetTopicSort(topicSort) {
this.conditionalSetState({topicSort});
}
currentMode() {
return this.state.mode;
}
Expand Down Expand Up @@ -919,6 +922,8 @@ class ReaderPanel extends Component {
<TopicPage
tab={this.state.tab}
setTab={this.setTab}
onSetTopicSort={this.onSetTopicSort}
topicSort={this.state.topicSort}
topic={this.state.navigationTopic}
topicTitle={this.state.topicTitle}
interfaceLang={this.props.interfaceLang}
Expand Down
9 changes: 7 additions & 2 deletions static/js/TopicPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,8 @@ const useTabDisplayData = (translationLanguagePreference) => {

const TopicPage = ({
tab, topic, topicTitle, setTopic, setNavTopic, openTopics, multiPanel, showBaseText, navHome,
toggleSignUpModal, openDisplaySettings, setTab, openSearch, translationLanguagePreference, versionPref, topicTestVersion
toggleSignUpModal, openDisplaySettings, setTab, openSearch, translationLanguagePreference, versionPref,
topicTestVersion, onSetTopicSort, topicSort
}) => {
const defaultTopicData = {primaryTitle: topicTitle, tabs: {}, isLoading: true};
const [topicData, setTopicData] = useState(Sefaria.getTopicFromCache(topic, {with_html: true}) || defaultTopicData);
Expand Down Expand Up @@ -505,6 +506,8 @@ const TopicPage = ({
}}
initialRenderSize={(topicData._refsDisplayedByTab && topicData._refsDisplayedByTab[key]) || 0}
renderItem={renderWrapper(toggleSignUpModal, topicData, topicTestVersion)}
onSetTopicSort={onSetTopicSort}
topicSort={topicSort}
/>
);
})
Expand Down Expand Up @@ -553,7 +556,7 @@ TopicPage.propTypes = {

const TopicPageTab = ({
data, renderItem, classes, sortOptions, sortFunc, filterFunc, showFilterHeader,
scrollableElement, onDisplayedDataChange, initialRenderSize
scrollableElement, onDisplayedDataChange, initialRenderSize, onSetTopicSort, topicSort
}) => {
return (
<div className="topicTabContents">
Expand All @@ -571,6 +574,8 @@ const TopicPageTab = ({
sortOptions={sortOptions}
onDisplayedDataChange={onDisplayedDataChange}
initialRenderSize={initialRenderSize}
onSetSort={onSetTopicSort}
externalSortOption={topicSort}
data={data}
/>
</div> : <LoadingMessage />
Expand Down

0 comments on commit ae6899b

Please sign in to comment.