Added tagged passage and story count display to Passage and Story Tags Dialogs #1498
Conversation
* display passageTag and storyTag frequencies Co-authored-by: sihambyte <[email protected]>
| @@ -9,5 +9,6 @@ export const TagEditor: React.FC<TagEditorProps> = props => ( | |||
| <button onClick={() => props.onChangeName('mock-new-name')}> | |||
| onChangeName | |||
| </button> | |||
| <span>{`${props.count} ${props.count > 1 ? 'passages' : 'passage'}`}</span> | |||
There was a problem hiding this comment.
Since this is a mock, let's just do
<span data-testid="count">{props.count}</span>| @@ -52,6 +53,7 @@ export const TagEditor: React.FC<TagEditorProps> = props => { | |||
| > | |||
| {t('common.color')} | |||
| </TextSelect> | |||
| <span className="tag-count">{`${count} ${count > 1 ? 'passages' : 'passage'}`}</span> | |||
There was a problem hiding this comment.
This text needs to be localized instead of hardcoding English. Take a look at StoryListRoute to see an example of how it handles plurals in the title. Putting just an English translation of the text in this PR is fine; I notify translators before a release so they can update the text.
I'd suggest using the key components.tagEditor.passageCount for this.
Tests will need to be updated too--we don't need to test that the localization works correctly, just that the right keys are displayed.
| @@ -24,7 +25,7 @@ export const PassageTagsDialog: React.FC<PassageTagsDialogProps> = props => { | |||
|
|
|||
| const story = storyWithId(stories, storyId); | |||
| const tags = storyPassageTags(story); | |||
|
|
|||
| const tagCountsInStory = passageTagFrequencies(story); | |||
There was a problem hiding this comment.
This should be memoized for performance reasons.
| @@ -17,6 +17,7 @@ export const StoryTagsDialog: React.FC<StoryTagsDialogProps> = props => { | |||
| const {t} = useTranslation(); | |||
|
|
|||
| const tags = storyTags(stories); | |||
| const tagCountsInStories = storyTagFrequencies(stories); | |||
| export function passageTagFrequencies(story: Story) { | ||
| const tagPassageCounts: { [key: string]: number } = {}; | ||
|
|
||
| story.passages.forEach(passage => { | ||
| if (passage.tags) { | ||
| passage.tags.forEach(tag=>{ | ||
| if (!tagPassageCounts[tag]) { | ||
| tagPassageCounts[tag] = 1; | ||
| } else { | ||
| tagPassageCounts[tag] += 1; | ||
| } | ||
| })}; | ||
| }); | ||
| return tagPassageCounts; | ||
| } | ||
|
|
There was a problem hiding this comment.
More idiomatically (and to parallel the name with storyPassageTags above it):
export function storyPassageTagFrequencies(story: Story) {
const counts: Record<string, number> = {};
for (const passage of story.passages) {
for (const tag of passage.tags) {
counts[tag] = (counts[tag] ?? 0) + 1;
}
}
return counts;
}I'm not sure why you do a check on passage.tags not being defined--it should always be, but did you find that wasn't the case in practice?
| @@ -196,6 +212,20 @@ export function storyTags(stories: Story[]) { | |||
| ).sort(); | |||
| } | |||
|
|
|||
| export function storyTagFrequencies(stories: Story[]) { | |||
There was a problem hiding this comment.
Same suggestion as with passageTagFrequencies.
|
It's been a while since this PR has seen any activity, so I'm going to close. Feel free to re-open, but bear in mind the suggestions I made on this. |
Description
Issues Fixed
#1391
Credit
[x] We would like to be credited in the application as Siham Argaw and Ngoc Nguyen.
Presubmission Checklist
[X] We have read this project's CONTRIBUTING file and this PR follows the criteria laid out there.
[X] This contribution was created by us and we have the right to submit it under the GPL v3 license. (This is not a rights assignment.)
[X] We have read and agree to abide by this project's Code of Conduct.