-
Notifications
You must be signed in to change notification settings - Fork 294
Show version history in the channel publish side panel #5721
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.
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
base: unstable
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,244 @@ | ||
| <template> | ||
|
|
||
| <div class="channel-version-history"> | ||
| <div | ||
| v-if="!isExpanded" | ||
| class="toggle-section" | ||
| > | ||
| <KButton | ||
| appearance="basic-link" | ||
| :text="seeAllVersions$()" | ||
| @click="handleExpand" | ||
| /> | ||
| </div> | ||
| <div | ||
| v-if="isExpanded" | ||
| class="versions-container" | ||
| > | ||
| <div | ||
| v-if="isLoading && versions.length === 0" | ||
| class="loading" | ||
| > | ||
| <KCircularLoader /> | ||
| </div> | ||
|
|
||
| <div | ||
| v-else-if="error && versions.length === 0" | ||
| class="error" | ||
| > | ||
| {{ errorLoadingVersions$() }} | ||
| </div> | ||
|
|
||
| <div | ||
| v-else | ||
| class="versions-list" | ||
| > | ||
| <!-- Display each version --> | ||
| <div | ||
| v-for="version in versions" | ||
| :key="version.id" | ||
| class="version-item" | ||
| > | ||
| <span class="version-number"> | ||
| {{ versionLabel$({ version: version.version }) }} | ||
| </span> | ||
| <div | ||
| v-if="version.version_notes" | ||
| class="version-description" | ||
| > | ||
| <div class="description-label"> | ||
| {{ versionDescriptionLabel$() }} | ||
| </div> | ||
| <div class="description-body"> | ||
| {{ version.version_notes }} | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <!-- "Show more" button - shown when more versions available --> | ||
| <div | ||
| v-if="hasMore" | ||
| class="show-more-section" | ||
| > | ||
| <KButton | ||
| appearance="basic-link" | ||
| :text="showMore$()" | ||
| :disabled="isLoadingMore" | ||
| @click="handleShowMore" | ||
| /> | ||
| <KCircularLoader | ||
| v-if="isLoadingMore" | ||
| class="inline-loader" | ||
| /> | ||
| <div | ||
| v-if="error && versions.length > 0" | ||
| class="fetch-more-error" | ||
| > | ||
| {{ errorLoadingVersions$() }} | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="collapse-section"> | ||
| <KButton | ||
| appearance="basic-link" | ||
| :text="seeLess$()" | ||
| @click="handleCollapse" | ||
| /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| </template> | ||
|
|
||
|
|
||
| <script> | ||
|
|
||
| import { ref, watch } from 'vue'; | ||
| import { useChannelVersionHistory } from 'shared/composables/useChannelVersionHistory'; | ||
| import { communityChannelsStrings } from 'shared/strings/communityChannelsStrings'; | ||
|
|
||
| export default { | ||
| name: 'ChannelVersionHistory', | ||
| setup(props) { | ||
| const isExpanded = ref(false); | ||
| const { | ||
| seeAllVersions$, | ||
| seeLess$, | ||
| showMore$, | ||
| versionLabel$, | ||
| errorLoadingVersions$, | ||
| versionDescriptionLabel$, | ||
| } = communityChannelsStrings; | ||
|
|
||
| const { | ||
| versions, | ||
| isLoading, | ||
| isLoadingMore, | ||
| error, | ||
| hasMore, | ||
| fetchVersions, | ||
| fetchMore, | ||
| reset, | ||
| } = useChannelVersionHistory(); | ||
|
|
||
| watch( | ||
| () => props.channelId, | ||
| (newChannelId, oldChannelId) => { | ||
| if (newChannelId !== oldChannelId) { | ||
| reset(); | ||
| isExpanded.value = false; | ||
| } | ||
| }, | ||
| ); | ||
|
|
||
| const handleExpand = async () => { | ||
| isExpanded.value = true; | ||
| if (versions.value.length === 0) { | ||
| await fetchVersions(props.channelId); | ||
| } | ||
| }; | ||
| const handleCollapse = () => { | ||
| isExpanded.value = false; | ||
| }; | ||
| const handleShowMore = async () => { | ||
| await fetchMore(); | ||
| }; | ||
|
|
||
| return { | ||
| isExpanded, | ||
| versions, | ||
| isLoading, | ||
| isLoadingMore, | ||
| error, | ||
| hasMore, | ||
| handleExpand, | ||
| handleCollapse, | ||
| handleShowMore, | ||
| seeAllVersions$, | ||
| seeLess$, | ||
| showMore$, | ||
| versionLabel$, | ||
| errorLoadingVersions$, | ||
| versionDescriptionLabel$, | ||
| }; | ||
| }, | ||
| props: { | ||
| channelId: { | ||
| type: String, | ||
| required: true, | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| </script> | ||
|
|
||
|
|
||
| <style scoped lang="scss"> | ||
|
|
||
| .channel-version-history { | ||
| margin-top: 16px; | ||
|
|
||
| .toggle-section { | ||
| margin: 8px 0; | ||
| } | ||
|
|
||
| .versions-container { | ||
| .loading, | ||
| .error { | ||
| padding: 8px 0; | ||
| text-align: center; | ||
| } | ||
|
|
||
| .versions-list { | ||
| .version-item { | ||
| padding: 12px 0; | ||
| border-bottom: 1px solid #e0e0e0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Several hardcoded color values here ( |
||
|
|
||
| .version-number { | ||
| font-weight: 500; | ||
| } | ||
|
|
||
| .version-description { | ||
| padding: 12px; | ||
| margin-top: 8px; | ||
| background-color: #f5f5f5; | ||
| border-radius: 6px; | ||
|
|
||
| .description-label { | ||
| margin-bottom: 6px; | ||
| font-weight: 600; | ||
| } | ||
|
|
||
| .description-body { | ||
| line-height: 1.4; | ||
| color: #4a4a4a; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| .show-more-section { | ||
| display: flex; | ||
| gap: 8px; | ||
| align-items: center; | ||
| margin: 8px 0; | ||
|
|
||
| .inline-loader { | ||
| width: 20px; | ||
| height: 20px; | ||
| } | ||
|
|
||
| .fetch-more-error { | ||
| font-size: 14px; | ||
| color: #d32f2f; | ||
| } | ||
| } | ||
|
|
||
| .collapse-section { | ||
| margin: 8px 0; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| </style> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -391,4 +391,9 @@ describe('PublishSidePanel', () => { | |
| await fireEvent.click(cancelBtn); | ||
| expect(emitted().close).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('renders ChannelVersionHistory component', () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: This test verifies |
||
| renderComponent(); | ||
| expect(screen.getByText(communityChannelsStrings.seeAllVersions$())).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| import { ref } from 'vue'; | ||
| import { ChannelVersion } from 'shared/data/resources'; | ||
|
|
||
| export const VERSIONS_PER_PAGE = 10; | ||
|
|
||
| /** | ||
| * Composable that fetches and manages paginated channel versions | ||
| * | ||
| * @returns {{ | ||
| * versions: import('vue').Ref<Array<Object>>, | ||
| * isLoading: import('vue').Ref<boolean>, | ||
| * isLoadingMore: import('vue').Ref<boolean>, | ||
| * error: import('vue').Ref<Error|null>, | ||
| * hasMore: import('vue').Ref<boolean>, | ||
| * currentPage: import('vue').Ref<number>, | ||
| * fetchVersions: (channelId: string) => Promise<void>, | ||
| * fetchMore: () => Promise<void>, | ||
| * }} | ||
| */ | ||
| export function useChannelVersionHistory() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. praise: Clean separation of concerns. The composable encapsulates pagination state, loading states, and error handling well. The |
||
| const versions = ref([]); | ||
| const isLoading = ref(false); | ||
| const isLoadingMore = ref(false); | ||
| const error = ref(null); | ||
| const hasMore = ref(false); | ||
| const currentPage = ref(0); | ||
| const currentChannelId = ref(null); | ||
|
|
||
| /** | ||
| * Fetch first page of versions for a channel | ||
| * @param {string} channelId - The channel ID to fetch versions for | ||
| */ | ||
| async function fetchVersions(channelId) { | ||
| isLoading.value = true; | ||
| error.value = null; | ||
| versions.value = []; | ||
| currentPage.value = 0; | ||
| currentChannelId.value = channelId; | ||
|
|
||
| try { | ||
| const response = await ChannelVersion.fetchCollection({ | ||
| channel: channelId, | ||
| page_size: VERSIONS_PER_PAGE, | ||
| page: 1, | ||
| }); | ||
|
|
||
| versions.value = response.results || []; | ||
| currentPage.value = 1; | ||
|
|
||
| // Check if there are more pages | ||
| // response.next will be null if no more pages | ||
| hasMore.value = response.next !== null; | ||
| } catch (err) { | ||
| error.value = err; | ||
| versions.value = []; | ||
| } finally { | ||
| isLoading.value = false; | ||
| } | ||
| } | ||
|
|
||
| async function fetchMore() { | ||
| if (!hasMore.value || isLoadingMore.value || !currentChannelId.value) { | ||
| return; | ||
| } | ||
|
|
||
| isLoadingMore.value = true; | ||
| error.value = null; | ||
|
|
||
| try { | ||
| const nextPage = currentPage.value + 1; | ||
| const response = await ChannelVersion.fetchCollection({ | ||
| channel: currentChannelId.value, | ||
| page_size: VERSIONS_PER_PAGE, | ||
| page: nextPage, | ||
| }); | ||
|
|
||
| versions.value = [...versions.value, ...(response.results || [])]; | ||
| currentPage.value = nextPage; | ||
|
|
||
| hasMore.value = response.next !== null; | ||
| } catch (err) { | ||
| error.value = err; | ||
| } finally { | ||
| isLoadingMore.value = false; | ||
| } | ||
| } | ||
|
|
||
| function reset() { | ||
| versions.value = []; | ||
| isLoading.value = false; | ||
| isLoadingMore.value = false; | ||
| error.value = null; | ||
| hasMore.value = false; | ||
| currentPage.value = 0; | ||
| currentChannelId.value = null; | ||
| } | ||
|
|
||
| return { | ||
| versions, | ||
| isLoading, | ||
| isLoadingMore, | ||
| error, | ||
| hasMore, | ||
| currentPage, | ||
| fetchVersions, | ||
| fetchMore, | ||
| reset, | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick:
version.version_notesis rendered safely via text interpolation ({{ }}), which is good. Just noting that if notes can be long, you may want to consider truncation oroverflow-wrap: break-wordon.description-bodyto handle edge cases with very long unbroken strings.