Skip to content

Commit 9c2d56b

Browse files
committed
fix(review): address PR #2079 review feedback
Address review comments from @joanagmaia, copilot-pull-request-reviewer: - geographical-distribution.vue: mount the all-countries drawer with v-if instead of client-only so its useQuery observer stops running once the drawer is closed (per copilot-pull-request-reviewer) - geo-map.helper.ts: extract the shared "hide Unknown" filter used by the widget, the drawer, and the shared view (previously duplicated) - geographical-distribution-drawer.vue: compute isEmpty over the known-countries filter so a response containing only "Unknown" shows "No data available" instead of an empty map/list (per copilot-pull-request-reviewer and @joanagmaia) - geographical-distribution-drawer.vue: add pb-5 to the scrollable section for spacing below the last row (per @joanagmaia) - geo-distribution-view.vue: format percentage with formatNumber, showing 2 decimals only below 1% (per @joanagmaia) Resolves 4 review threads. Signed-off-by: Efren Lim <elim@linuxfoundation.org>
1 parent cf51a04 commit 9c2d56b

4 files changed

Lines changed: 25 additions & 14 deletions

File tree

frontend/app/components/modules/widget/config/contributor/geographical-distribution/fragments/geo-distribution-view.vue

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ SPDX-License-Identifier: MIT
2626
</span>
2727
</div>
2828
<span>
29-
{{ formatNumber(item.count) }} {{ pluralize(props.label.toLowerCase(), item.count) }} ・ {{ item.percentage }}%
29+
{{ formatNumber(item.count) }} {{ pluralize(props.label.toLowerCase(), item.count) }} ・
30+
{{ formatNumber(item.percentage, item.percentage < 1 ? 2 : 0) }}%
3031
</span>
3132
</div>
3233
</div>
@@ -35,6 +36,7 @@ SPDX-License-Identifier: MIT
3536
<script setup lang="ts">
3637
import { computed } from 'vue';
3738
import pluralize from 'pluralize';
39+
import { filterKnownCountries } from '../geo-map.helper';
3840
import LfxChart from '~/components/uikit/chart/chart.vue';
3941
import { convertToChartData, getMaxValue } from '~/components/uikit/chart/helpers/chart-helpers';
4042
import type { ChartData, RawChartData, ChartSeries } from '~/components/uikit/chart/types/ChartTypes';
@@ -56,8 +58,7 @@ const props = withDefaults(
5658
},
5759
);
5860
59-
// "Unknown" location is intentionally hidden from the list and chart (see IN-1225 / #2062).
60-
const knownGeoMapData = computed(() => props.geoMapData.filter((item) => item.name !== 'Unknown'));
61+
const knownGeoMapData = computed(() => filterKnownCountries(props.geoMapData));
6162
6263
const listData = computed(() => (props.limit ? knownGeoMapData.value.slice(0, props.limit) : knownGeoMapData.value));
6364

frontend/app/components/modules/widget/config/contributor/geographical-distribution/fragments/geographical-distribution-drawer.vue

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ SPDX-License-Identifier: MIT
2424

2525
<hr />
2626
</div>
27-
<section class="mt-5 flex flex-col flex-grow overflow-auto">
27+
<section class="mt-5 flex flex-col flex-grow overflow-auto pb-5">
2828
<div class="flex flex-wrap gap-4 items-center justify-between mb-6 px-4 sm:px-6 pt-[1px]">
2929
<lfx-tabs
3030
:tabs="tabs"
@@ -66,6 +66,7 @@ SPDX-License-Identifier: MIT
6666
import { useRoute } from 'nuxt/app';
6767
import { ref, computed, watch } from 'vue';
6868
import { storeToRefs } from 'pinia';
69+
import { filterKnownCountries } from '../geo-map.helper';
6970
import LfxGeoDistributionView from './geo-distribution-view.vue';
7071
import LfxDrawer from '~/components/uikit/drawer/drawer.vue';
7172
import LfxTabs from '~/components/uikit/tabs/tabs.vue';
@@ -129,7 +130,9 @@ const { data, status, error } = CONTRIBUTORS_API_SERVICE.fetchGeographicalDistri
129130
130131
const geoMapData = computed<GeoMapData[] | undefined>(() => (data.value as GeoMapResponse)?.data);
131132
132-
const isEmpty = computed(() => isEmptyData(geoMapData.value as unknown as Record<string, unknown>[]));
133+
const isEmpty = computed(() =>
134+
isEmptyData(filterKnownCountries(geoMapData.value) as unknown as Record<string, unknown>[]),
135+
);
133136
134137
const tabs = [
135138
{
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Copyright (c) 2025 The Linux Foundation and each contributor.
2+
// SPDX-License-Identifier: MIT
3+
import type { GeoMapData } from '~/components/modules/widget/components/contributors/types/geo-map.types';
4+
5+
// "Unknown" location is intentionally hidden from the list and chart (see IN-1225 / #2062).
6+
export const filterKnownCountries = (data?: GeoMapData[]): GeoMapData[] =>
7+
data?.filter((item) => item.name !== 'Unknown') ?? [];

frontend/app/components/modules/widget/config/contributor/geographical-distribution/geographical-distribution.vue

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,13 @@ SPDX-License-Identifier: MIT
5656
</div>
5757
</lfx-project-load-state>
5858
</section>
59-
<client-only>
60-
<lfx-geographical-distribution-drawer
61-
v-model="isDrawerOpened"
62-
:selected-tab="model.activeTab"
63-
:selected-metric="model.metric"
64-
:model="model"
65-
/>
66-
</client-only>
59+
<lfx-geographical-distribution-drawer
60+
v-if="isDrawerOpened"
61+
v-model="isDrawerOpened"
62+
:selected-tab="model.activeTab"
63+
:selected-metric="model.metric"
64+
:model="model"
65+
/>
6766
</template>
6867

6968
<script setup lang="ts">
@@ -72,6 +71,7 @@ import { useRoute } from 'vue-router';
7271
import { storeToRefs } from 'pinia';
7372
import LfxGeoDistributionView from './fragments/geo-distribution-view.vue';
7473
import LfxGeographicalDistributionDrawer from './fragments/geographical-distribution-drawer.vue';
74+
import { filterKnownCountries } from './geo-map.helper';
7575
import LfxTabs from '~/components/uikit/tabs/tabs.vue';
7676
import LfxButton from '~/components/uikit/button/button.vue';
7777
import { useProjectStore } from '~/components/modules/project/store/project.store';
@@ -131,7 +131,7 @@ const { data, status, error } = CONTRIBUTORS_API_SERVICE.fetchGeographicalDistri
131131
const isDrawerOpened = ref(false);
132132
133133
const geoMapData = computed<GeoMapData[] | undefined>(() => (data.value as GeoMapResponse)?.data);
134-
const knownGeoMapData = computed(() => geoMapData.value?.filter((item) => item.name !== 'Unknown') ?? []);
134+
const knownGeoMapData = computed(() => filterKnownCountries(geoMapData.value));
135135
const showAllCountriesButton = computed(() => knownGeoMapData.value.length > 5);
136136
137137
const isEmpty = computed(() => isEmptyData(knownGeoMapData.value as unknown as Record<string, unknown>[]));

0 commit comments

Comments
 (0)