Skip to content

Improve schema link handling in SubjectEditor#438

Merged
JeroenDeDauw merged 3 commits into
masterfrom
feat-430
Jan 22, 2026
Merged

Improve schema link handling in SubjectEditor#438
JeroenDeDauw merged 3 commits into
masterfrom
feat-430

Conversation

@alistair3149

@alistair3149 alistair3149 commented Jan 21, 2026

Copy link
Copy Markdown
Member

For #430

With permission
image

Without permission
image

@alistair3149 alistair3149 Jan 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is used for cases where we want to inject the whole Vue slot into parameters in i18n messages, when mw.message.parse() does not cut it. In this example, we want to retain the event handling for the schema link.

@malberts malberts Jan 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you consider alternative approaches? I don't have anything specific in mind, but this feels like introducing complication for the sake of doing everything/too much in a single message string.

Where else would we use this? Only if we follow this same design pattern of having a dynamic string, which does not behave like a normal link, in another message string?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I considered attaching the event listener manually in JS every time when the dialog is opened. It'll end up with a similar level of complexity.

We need to put the syntax label: value into the same i18n message to ensure proper localization. Or perhaps there's a better way to handle this.

Where else would we use this?

Anywhere that we need to inject Vue component in place of a parameter for i18n message. The inline button (e.g. link button) would need that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alistair and I briefly talked about this

Notes from me:

  • Better to have done the trivial bad-i18n lego approach in this commit
  • Then either create issue for discussion and priotization of fixing, or make a dedicated PR right away
  • For prototype we do not need to have proper i18n everywhere. This is something we can fix later. Fixing the last few % here does not help us testing product fit and gathering feedback

@@ -22,3 +22,19 @@ export function createTestWrapper<TComponent extends DefineComponent<any, any, a
},
) as VueWrapper<InstanceType<TComponent>>;
}

export function mockMwMessage(

@alistair3149 alistair3149 Jan 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

A bit of YAGNI but we will likely use mw.message in multiple tests, so it is worthwhile to put that into a helper function. It is rather barebone now (i.e. only covering mw.message.text), but we can expand it as needed.

@alistair3149
alistair3149 marked this pull request as ready for review January 21, 2026 20:58
@alistair3149
alistair3149 requested review from JeroenDeDauw and malberts and removed request for JeroenDeDauw January 21, 2026 20:58
@JeroenDeDauw
JeroenDeDauw requested a review from Copilot January 21, 2026 22:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the schema link handling in SubjectEditor to conditionally display the schema name as either a clickable link (when the user has edit permissions) or plain text (when they don't), improving the user experience by providing appropriate UI based on permissions.

Changes:

  • Created a new I18nSlot component to handle internationalized text with embedded slots
  • Refactored SubjectEditorDialog to use the new component with conditional rendering for schema name display
  • Updated i18n message keys, replacing neowiki-edit-schema with neowiki-schema-label for better clarity

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
resources/ext.neowiki/src/components/common/I18nSlot.vue New component for rendering internationalized text with embedded slots
resources/ext.neowiki/tests/components/common/I18nSlot.spec.ts Comprehensive test coverage for the I18nSlot component
resources/ext.neowiki/tests/VueTestHelpers.ts Added mockMwMessage helper function for testing MediaWiki message handling
resources/ext.neowiki/src/components/SubjectEditor/SubjectEditorDialog.vue Refactored to use I18nSlot with conditional rendering based on schema edit permissions
i18n/en.json Added neowiki-schema-label message and removed neowiki-edit-schema
i18n/qqq.json Added documentation for the new neowiki-schema-label message
extension.json Updated message references to include neowiki-schema-label and remove neowiki-edit-schema

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JeroenDeDauw
JeroenDeDauw merged commit dee2b4e into master Jan 22, 2026
15 checks passed
@JeroenDeDauw
JeroenDeDauw deleted the feat-430 branch January 22, 2026 16:04
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.

4 participants