Skip to content

Rewrite #17

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

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft

Rewrite #17

wants to merge 19 commits into from

Conversation

mProjectsCode
Copy link
Owner

Rewrites the data collection to Rust and switches plotting lib.

Copy link
Contributor

@Fevol Fevol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

</table>

<p class="muted">
<i>Blacklisted</i> versions are versions that are known to cause major issues or contain security concerns and have thus been blacklisted by the Obsidian team.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be hidden unless there is actually a deprecated version.

<Line data={mappedData} x="date" y="downloads" stroke="id" marker={showDots ? 'dot' : undefined} />
<Pointer data={mappedData} x="date" z="id" maxDistance={30}>
{#snippet children({ data })}
<Text {data} fill="id" x="date" y="downloads" text={d => d.downloads.toFixed()} lineAnchor="bottom" dy={-7} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Applies to all graphs)
It would be nice if the tooltip had some kind of semi-transparent background, visibility is sometimes kind of poor

<Line data={mappedDataPoints} x="date" y="total_with_removed" stroke={totalWithRemovedStroke} marker="dot" />
<Pointer data={mappedDataPoints} x="date" y="total" maxDistance={30}>
{#snippet children({ data })}
<Text {data} fill={totalStroke} x="date" y="total" text={d => d.total.toFixed()} lineAnchor="bottom" dy={-7} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently only shows the tooltip for the 'total plugin count', I think it might be useful to also show a tooltip for the 'plugin count with removed plugin' field at the same time, like the charts in the hall of fame page

};
});

const totalStroke = `Total ${typeToString(type, false, true)} Count`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Active plugin count? Listed plugin count?


const { dataPoints, type }: Props = $props();

const mappedDataPoints = dataPoints
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is just on my end, but the chart includes the August month with 0 plugins. In this case, it might be better to just exclude it from the view?

@@ -106,35 +40,28 @@ const limitaionsLabels = Object.keys(licenseLimitations).map(x => licenses.descr
<p>The following chart shows the distribution of licenses in the plugins.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Applies to the description above, apparently I cannot comment on it)
"Another developer might want to continue development of an abandoned plugin, but if he is allowed depends on the license."

Should be:
"Another developer might want to continue development of an abandoned plugin, but whether they are allowed to, depends on the license."

percent100={totalPluginCount}
client:only="svelte"
/>
<BarChart dataPoints={licenseDataPoints.licenses} xLabel="License" yLabel="Number of Plugins" skewLabels hideBarValues client:only="svelte" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it would make more sense to have this charts be percentages, rather than absolute amounts, just like the repository data. Idem for all subsequent barcharts.

(Or use absolute values instead of percentages for repository data)

@@ -1,7 +1,12 @@
---
import StarlightPage from '@astrojs/starlight/components/StarlightPage.astro';
import ThemesIndex from '../../components/svelte/indexPages/themesIndex.svelte';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't test, got a WASM error when loading the page, and I think theme required data was not included. I tried but failed to fetch the theme data myself (I assume I had to clone all themes first?)

If it helps, I got this trace:

Empty theme list at commit 2b64fc20c968b850288bfbc2b99f843f1d7f7f43 (2024-10-31)
Empty theme list at commit 9cbe6f390717d90c4fb6d834121f987cf6ad2184 (2024-04-05)
Empty theme list at commit 9333f0e4b144ae9af0431a7a3916cf7ca6ea8787 (2023-12-29)
Get theme lists: 64.4503ms
Building theme data...

thread 'main' panicked at src\theme\data.rs:64:5:
No theme lists found

(For now, I'll assume the theme pages look great! 😄)

<h3 id="changelog-changes">Changelog Changes</h3>

<p>
Each Obsidian release has a changelog, which lists the changes made in that release. The following chart shows the number of changes made in each release,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is important that we note how we determine 'the amount of changes made in that release'.

In the old version, this was:

This statistic is determined by simply counting the number of items in the changelog.

Also, the changelog of major public versions can include changes from all the previous insider versions.

const overview = dataArray.get_changelog_overview();
const changelogChanges = dataArray.get_changelog_changes_for_minor_releases();
const changelogChangesDataPoints = dataArray.get_changelog_changes_as_data_points();
const changelogCategories = dataArray.get_changelog_categories();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking Changes is joined together in the table (as BreakingChanges)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants