Skip to content

Fix doc wording/clarity and stabilize Playground date/link identifiers#448

Open
srozen with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-grammar-and-spelling-errors
Open

Fix doc wording/clarity and stabilize Playground date/link identifiers#448
srozen with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-grammar-and-spelling-errors

Conversation

Copilot AI commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

This PR applies the requested copy and behavior fixes across Django and Vue surfaces: grammar/spelling corrections in docs, clearer guidance for nested container depth, and deterministic demo logic updates to avoid future-dated defaults and duplicate link IDs.

  • Django package description

    • Updated the nside_wefa module docstring to use a grammatically complete list (..., data protection, and authentication.).
  • Router container Storybook docs

    • Corrected wording in overview.mdx (multiplesmultiple).
    • Added explicit depth semantics for nested containers so consumers can map route nesting level to depth values (1, 2, ...).
  • Playground Gantt demo behavior

    • Replaced hard-coded 2026 top-level range dates with currentYear-based dates.
    • Made link IDs unique by deriving them from source index, target index, and type to prevent collisions when multiple links share the same type.
const currentYear = new Date().getFullYear()
const startDate = new Date(currentYear, 0, 1)
const endDate = new Date(currentYear, 1, 28)

id: `${linkCase.fromIndex}-${linkCase.toIndex}-${linkCase.type}`
Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The sentence is grammatically incomplete. Consider adding 'and' before 'authentication' to make it a proper list: 'with a focus on Legal Consent compliance, data protection, and authentication.'","fixFiles":[{"filePath":"django/nside_wefa/__init__.py","diff":"diff --git a/django/nside_wefa/__init__.py b/django/nside_wefa/__init__.py\n--- a/django/nside_wefa/__init__.py\n+++ b/django/nside_wefa/__init__.py\n@@ -2,7 +2,7 @@\n nside-demo: Django library with utilities and reusable apps.\n \n This package provides Django applications and utilities for web development,\n-with a focus on Legal Consent compliance, data protection, authentication.\n+with a focus on Legal Consent compliance, data protection, and authentication.\n \"\"\"\n \n __version__ = \"0.6.2\"\n"}]},{"message":"Corrected spelling of 'multiples' to 'multiple'.","fixFiles":[{"filePath":"vue/src/containers/storybook/overview.mdx","diff":"diff --git a/vue/src/containers/storybook/overview.mdx b/vue/src/containers/storybook/overview.mdx\n--- a/vue/src/containers/storybook/overview.mdx\n+++ b/vue/src/containers/storybook/overview.mdx\n@@ -20,7 +20,7 @@\n \n **If one of your route has children routes, you MUST use a Container Component on it.** Router depth tracking depends on it.\n \n-Let's take the example of a settings panel which contains multiples views for password reset, api token and user preferences :\n+Let's take the example of a settings panel which contains multiple views for password reset, api token and user preferences :\n \n ```ts\n /** router.ts*/\n"}]},{"message":"The `depth` prop is set to 2 in the nested container example, but there's no explanation of what depth values mean or when to use different depths. This could confuse users trying to implement nested containers.","fixFiles":[{"filePath":"vue/src/containers/storybook/overview.mdx","diff":"diff --git a/vue/src/containers/storybook/overview.mdx b/vue/src/containers/storybook/overview.mdx\n--- a/vue/src/containers/storybook/overview.mdx\n+++ b/vue/src/containers/storybook/overview.mdx\n@@ -191,6 +191,9 @@\n You can also have deeper hierarchy, with children of children. This is useful when one section owns a\n second local navigation level.\n \n+`depth` indicates the container level in the nested route tree: use `depth: 1` for a top-level\n+container, `depth: 2` for a container nested inside it, and so on.\n+\n ```ts\n /** router.ts*/\n \n"}]},{"message":"Hard-coded dates use the year 2026, which is in the future. Consider using a relative date calculation or documenting why this specific future date range is used for the demo.","fixFiles":[{"filePath":"vue/src/demo/views/PlaygroundView.vue","diff":"diff --git a/vue/src/demo/views/PlaygroundView.vue b/vue/src/demo/views/PlaygroundView.vue\n--- a/vue/src/demo/views/PlaygroundView.vue\n+++ b/vue/src/demo/views/PlaygroundView.vue\n@@ -39,8 +39,9 @@\n \n const { t } = useI18nLib()\n \n-const startDate = new Date(2026, 0, 1)\n-const endDate = new Date(2026, 1, 28)\n+const currentYear = new Date().getFullYear()\n+const startDate = new Date(currentYear, 0, 1)\n+const endDate = new Date(currentYear, 1, 28)\n const okLinkClass = '[stroke-dasharray:2,5]'\n const linkCases = [\n   { type: 'start-start', fromIndex: 2, toIndex: 0, color: 'green', class: okLinkClass },\n"}]},{"message":"Using `linkCase.type` as the link id will cause duplicate IDs when multiple links share the same type (e.g., multiple 'start-start' links). This violates the uniqueness assumption for link IDs and could cause issues with link highlighting and identification. Consider using a unique identifier such as `${linkCase.fromIndex}-${linkCase.toIndex}-${linkCase.type}` or the array index.","fixFiles":[{"filePath":"vue/src/demo/views/PlaygroundView.vue","diff":"diff --git a/vue/src/demo/views/PlaygroundView.vue b/vue/src/demo/views/PlaygroundView.vue\n--- a/vue/src/demo/views/PlaygroundView.vue\n+++ b/vue/src/demo/views/PlaygroundView.vue\n@@ -188,7 +188,7 @@\n     )\n     if (from?.id && to?.id) {\n       linkPairs.push({\n-        id: linkCase.type,\n+        id: `${linkCase.fromIndex}-${linkCase.toIndex}-${linkCase.type}`,\n         fromId: from.id,\n         toId: to.id,\n         type: linkCase.type,\n"}]}]

Copilot AI changed the title [WIP] Fix grammar and spelling errors in documentation Fix doc wording/clarity and stabilize Playground date/link identifiers Jul 3, 2026
Copilot finished work on behalf of srozen July 3, 2026 13:18
Copilot AI requested a review from srozen July 3, 2026 13:18
@srozen

srozen commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@codex review

@srozen srozen marked this pull request as ready for review July 3, 2026 13:22
@srozen srozen requested a review from fsouris July 3, 2026 13:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a3d496230

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +194 to +195
`depth` indicates the container level in the nested route tree: use `depth: 1` for a top-level
container, `depth: 2` for a container nested inside it, and so on.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Describe depth as recursion depth, not route level

This added explanation says depth is the container's nesting level, but the container code passes it to menuItemsFromRoute(..., depth), where it controls how many descendant route levels to include from the current container (1 is immediate children). A top-level SideMenuContainer that should include grandchildren therefore needs depth: 2—as the example below does—so following this text would hide nested entries rather than configure nested containers correctly.

Useful? React with 👍 / 👎.

Comment on lines +43 to +44
const startDate = new Date(currentYear, 0, 1)
const endDate = new Date(currentYear, 1, 28)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Align Gantt bounds with generated activity dates

When this demo is opened in any year other than 2026, these bounds use currentYear, but every generated row/activity/fill segment below still uses new Date(2026, ...). Since GanttChartComponent builds its visible dateRange from startDate/endDate, the playground will show a non-2026 calendar while rendering 2026 activities clamped into that grid, making hover dates and link validation misleading. Derive all activity dates from the same year or keep the range fixed/documented.

Useful? React with 👍 / 👎.

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.

3 participants