Translate: show where linked strings live when active filters don't match#4281
Translate: show where linked strings live when active filters don't match#4281ayshushus wants to merge 4 commits into
Conversation
|
TODO
|
eemeli
left a comment
There was a problem hiding this comment.
Looks decent, though I only did a surface-level review pass here.
| ), | ||
| } | ||
| if requested_entity_location is not None: | ||
| response["requested_entity_location"] = requested_entity_location |
There was a problem hiding this comment.
requested_entity_location feels like a pretty verbose name for this in the API, why not just requested_entity?
| parts.push('other filters'); | ||
| } | ||
| return parts.join(', '); | ||
| } |
There was a problem hiding this comment.
This should be localized, and the concatenation should use Intl.ListFormat.
| {`String ${string} is in ${stringResource} (${stringProject}). ` + | ||
| (filters | ||
| ? `You're browsing ${queryLabel}, filtered by ${filters}.` | ||
| : `You're browsing ${queryLabel}.`)} |
There was a problem hiding this comment.
We should really be able to rely on our own localization system, rather than duplicating the English strings all over the place.
| } | ||
|
|
||
| const { push } = location; | ||
| const string = String(entityLocation.pk); |
There was a problem hiding this comment.
This should probably be called something like stringId.
| show: entity > 0 && requestedEntityLocation != null && !found, | ||
| entityLocation: requestedEntityLocation, |
There was a problem hiding this comment.
Why can't we rely only on whether requestedEntityLocation is empty?
| const stringNotFound = | ||
| selectedEntity > 0 && !isValid && requestedEntityLocation != null; |
There was a problem hiding this comment.
Also here, why not only check requestedEntityLocation? The check right below also uses !selectedEntity, and it's not clear why all of these conditions matter.
There was a problem hiding this comment.
I think it'd be useful for e.g. @flodolo to look over these strings.
| ## String not found page | ||
| ## Shown when the requested string doesn't match the rest of the query | ||
|
|
||
| entities-StringNotFound--description = |
There was a problem hiding this comment.
This would never pass review in one of our projects. Variants should be fully formed strings, and variants are not guaranteed to exist in the localized version (they're private). So, this is not a good use of variants.
What's $filters? Can we support multiple strings and selection at run-time instead of variants?
It would be great if you could share a screenshot of how the page looks like.
Fix #2921
The
stringURL param permalinks a specific string, but shared links also carry the filters active at link time (status=missing, search, ...). When the target doesn't match those filters (e.g. link saysstatus=missingbut the string is now translated), we silently redirected to the first list entry + a toast, landing users on an unrelated string. Now surface where the string actually lives and let the user choose.Backend:
base.views._get_paginated_entities: when the requestedentityis filtered out, run a second, filter-free lookup gated only by viewability (obsolete=False,translatedresources__locale, project not disabled/system,Project.objects.visible_for(user)) and attachrequested_entity_location({pk, project slug, resource path}) to the response. Absent when the string is invalid/obsolete/not viewable, so behavior is unchanged there. Threaduserthrough for the visibility gate.State:
api/entity.ts: addRequestedEntityLocationtype, optionalrequested_entity_locationon the response.entities/actions.ts,reducer.ts: carry it ontoRECEIVE_ENTITIES, store asrequestedEntityLocation. Preserve across pages (backend only reports on page 1), clear onRESET.entities/hooks.ts:useEntityNotFound()→showwhenentity > 0 && requestedEntityLocation != null && !found.UI:
entities/components/StringNotFound.{tsx,css}: new editor-panel screen. "See string" →push({ ...emptyParams, project, resource, entity })(keep string, drop filters; navigates cross-project). "See other strings" →push({ entity: 0 })(keep filters, drop string).App.tsx: render<StringNotFound>over<Entity>/<BatchActions>whenuseEntityNotFound().show.entitieslist/EntitiesList.tsx: skip the "select first entity" redirect +ENTITY_NOT_FOUNDtoast when the string-not-found screen applies.context/Location.tsx: exportemptyParams(consumed byStringNotFound).locale/en-US/translate.ftl: addentities-StringNotFound--*strings.Tests:
test_entities_not_matching_string_reports_location: reports location when filtered out; absent when the string matches.StringNotFound.test.jsx: both navigation paths (incl. cross-project) + null-location renders nothing.