Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/core-web/dist/css/markbind.min.css

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/core-web/dist/css/vueCommonAppFactory.min.css

Large diffs are not rendered by default.

Binary file modified packages/core-web/dist/fonts/KaTeX_AMS-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_AMS-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_AMS-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Caligraphic-Bold.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Caligraphic-Bold.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Caligraphic-Bold.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Caligraphic-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Caligraphic-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Caligraphic-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Fraktur-Bold.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Fraktur-Bold.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Fraktur-Bold.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Fraktur-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Fraktur-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Fraktur-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-Bold.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-Bold.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-Bold.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-BoldItalic.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-BoldItalic.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-BoldItalic.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-Italic.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-Italic.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-Italic.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Main-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Math-BoldItalic.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Math-BoldItalic.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Math-BoldItalic.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Math-Italic.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Math-Italic.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Math-Italic.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_SansSerif-Bold.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_SansSerif-Bold.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_SansSerif-Bold.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_SansSerif-Italic.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_SansSerif-Italic.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_SansSerif-Italic.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_SansSerif-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_SansSerif-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_SansSerif-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Script-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Script-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Script-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size1-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size1-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size1-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size2-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size2-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size2-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size3-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size3-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size3-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size4-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size4-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Size4-Regular.woff2
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Typewriter-Regular.ttf
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Typewriter-Regular.woff
Binary file not shown.
Binary file modified packages/core-web/dist/fonts/KaTeX_Typewriter-Regular.woff2
Binary file not shown.
2 changes: 1 addition & 1 deletion packages/core-web/dist/js/markbind.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/core-web/dist/js/vueCommonAppFactory.min.js

Large diffs are not rendered by default.

58 changes: 41 additions & 17 deletions packages/core/src/html/includePanelProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,29 @@ export function processInclude(node: MbNode, context: Context, pageSources: Page
return childContext;
}

function createPopoverInlineErrorSlot(message: string): NodeOrText[] {
return [
{
type: 'tag',
name: 'template',
attribs: { slot: 'content' },
children: [
{
type: 'tag',
name: 'div',
attribs: { style: 'color: red;' },
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

[nitpick] Inline styles with hardcoded colors should use CSS classes instead for consistency and maintainability. Consider using a CSS class like 'error-message' that can be styled centrally.

Suggested change
attribs: { style: 'color: red;' },
attribs: { class: 'error-message' },

Copilot uses AI. Check for mistakes.
children: [
{
type: 'text',
data: message,
},
],
},
],
},
];
}

/**
* PreProcesses popovers with the src attribute.
* Replaces it with an error node if the specified src is invalid.
Expand All @@ -318,9 +341,11 @@ export function processPopoverSrc(node: MbNode, context: Context, pageSources: P
}

if (_.isEmpty(node.attribs.src)) {
const error = new Error(`Empty src attribute in popover in: ${context.cwf}`);
logger.error(error.message);
cheerio(node).replaceWith(createErrorNode(node, error));
// const error = new Error(`Empty src attribute in popover in: ${context.cwf}`);
const errorMsg = `Empty src attribute in popover in: ${context.cwf}`;
logger.error(errorMsg);
// cheerio(node).replaceWith(createErrorNode(node, error));
node.children = createPopoverInlineErrorSlot(errorMsg);
return context;
}

Expand All @@ -334,14 +359,13 @@ export function processPopoverSrc(node: MbNode, context: Context, pageSources: P

// No need to process url contents
if (isUrl) {
const error = new Error('URLs are not allowed in the \'src\' attribute');
logger.error(`${error.message}
File: ${context.cwf}
URL provided: ${node.attribs.src}

Please check the \`src\` attribute in the popover element.
Ensure it doesn't contain a URL (e.g., "http://www.example.com").`);
cheerio(node).replaceWith(createErrorNode(node, error));
const errorMsg
= 'URLs are not allowed in the \'src\' attribute.<br>'
+ `File: ${context.cwf}<br>`
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

Using HTML <br> tags in error messages couples the error content with presentation. Consider using \n for line breaks and let the display layer handle formatting, or use an array of message parts.

Suggested change
+ `File: ${context.cwf}<br>`
= 'URLs are not allowed in the \'src\' attribute.\n'
+ `File: ${context.cwf}\n`

Copilot uses AI. Check for mistakes.
+ `URL provided: ${node.attribs.src}`;

logger.error(errorMsg);
node.children = createPopoverInlineErrorSlot(errorMsg);
return context;
}

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it would be helpful to understand how the original implementation displays the error message in-place without touching the logic in the separate vue UI components

Expand Down Expand Up @@ -372,12 +396,12 @@ export function processPopoverSrc(node: MbNode, context: Context, pageSources: P
actualContent = $(hash).html() || '';

if (actualContent === '') {
const error = new Error(`No such segment '${hash}' in file: ${actualFilePath}\n`
+ `Missing reference in ${context.cwf}`);
logger.error(error.message);

cheerio(node).replaceWith(createErrorNode(node, error));
const errorMsg
= `No such segment '${hash}' in file: ${actualFilePath}\n`
+ `Missing reference in ${context.cwf}`;

logger.error(errorMsg);
node.children = createPopoverInlineErrorSlot(errorMsg);
return context;
}
}
Expand All @@ -390,7 +414,7 @@ export function processPopoverSrc(node: MbNode, context: Context, pageSources: P
if (childContext.hasExceededMaxCallstackSize()) {
const error = new CyclicReferenceError(childContext.callStack);
logger.error(error.message);
cheerio(node).replaceWith(createErrorNode(node, error));
node.children = createPopoverInlineErrorSlot(error.message);
return context;
}
}
Expand Down
74 changes: 71 additions & 3 deletions packages/vue-components/src/Popover.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
data-mb-component-type="popover"
tabindex="0"
>
<portal v-if="targetEl.id" :to="'popover:' + targetEl.id">

<div v-if="localError" class="popover-error-message">
{{ localError }}
</div>

<portal v-if="targetEl.id && !localError" :to="'popover:' + targetEl.id">
<h3 v-if="hasHeader" class="popover-header">
<slot name="header"></slot>
</h3>
Expand All @@ -13,7 +18,7 @@
</div>
</portal><!-- do not delete this comment, it is for the stray space issue (#2419)
--><v-popover
v-if="isMounted"
v-if="isMounted && !localError"
:auto-hide="!isInput"
:triggers="triggers"
:popper-triggers="triggers"
Expand Down Expand Up @@ -60,19 +65,28 @@ export default {
type: String,
default: 'top',
},
src: {
Copy link
Member

Choose a reason for hiding this comment

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

Should the popover expect this attribute?

const attributeSlotElement: NodeOrText[] = createSlotTemplateNode('content', actualContent);
node.children = node.children ? attributeSlotElement.concat(node.children) : attributeSlotElement;
delete node.attribs.src;
return childContext;
}

Our processPopoverSrc upstream function deletes this attribute, would it be better that the vue component remains unaware of a potential src attribute?

type: String,
default: '',
},
header: {
type: String,
default: '',
},
},
data() {
return {
targetEl: {},
isMounted: false,
localError: '',
};
},
computed: {
triggers() {
return this.trigger.split(' ');
},
hasHeader() {
return !!this.$slots.header;
return !!this.$slots.header || this.header;
},
isInput() {
return Boolean(this.$slots.default && this.$slots.default().some(vnode => vnode.type === 'input'));
Expand All @@ -81,6 +95,54 @@ export default {
mounted() {
this.targetEl = this.$el;
this.isMounted = true;
this.$nextTick(() => {
this.captureAndLocalizeErrors();
});
},
methods: {
captureAndLocalizeErrors() {
// Method 1: Check for common error patterns in the DOM
this.findAndHandleGlobalErrors();

// Method 2: Validate proactively
this.validatePopover();
},
findAndHandleGlobalErrors() {
// Look for error messages that might have been injected elsewhere
const popoverErrors = Array.from(document.querySelectorAll('.popover-error'))
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, where do the elements with class popover-error originate from? I can't seem to find where such elements would be injected or modified upstream

.filter(el => el.textContent.includes('popover') || el.textContent.includes('src'));

popoverErrors.forEach((errorEl) => {
if (this.$el.contains(errorEl)) {
// Error is already in the right place
return;
}

// Check if this error belongs to our popover
if (errorEl.textContent.includes(this.src)
|| errorEl.textContent.includes(this.header)) {
this.localError = errorEl.textContent;
errorEl.remove();
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

Directly removing DOM elements with errorEl.remove() can cause memory leaks and break Vue's reactivity. If error elements need to be removed, use Vue's conditional rendering with v-if instead.

Suggested change
errorEl.remove();
// Instead of removing the element, hide it to avoid breaking Vue's reactivity
errorEl.style.display = 'none';

Copilot uses AI. Check for mistakes.
}
});
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

Direct DOM querying with document.querySelectorAll breaks Vue's component encapsulation and reactive data flow. Consider using Vue's ref system or emitting events to parent components instead of global DOM manipulation.

Suggested change
});
// Check for error messages within this component only
const errorEl = this.$refs.popoverError;
if (errorEl && (errorEl.textContent.includes('popover') || errorEl.textContent.includes('src'))) {
// Optionally, further check if the error is related to this popover
if (errorEl.textContent.includes(this.src) || errorEl.textContent.includes(this.header)) {
this.localError = errorEl.textContent;
}
}

Copilot uses AI. Check for mistakes.
},
validatePopover() {
if (!this.src && !this.$slots.content) {
this.localError = 'Popover requires either a src attribute or content';
return;
}
if (this.src && !this.isValidUrl(this.src)) {
this.localError = 'Invalid popover source URL';
}
},
isValidUrl(url) {
try {
(() => new URL(url))();
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

The immediately invoked function expression (IIFE) pattern (() => new URL(url))() is unnecessarily complex. Replace with new URL(url) directly since the constructor already throws on invalid URLs.

Suggested change
(() => new URL(url))();
new URL(url);

Copilot uses AI. Check for mistakes.
return true;
} catch (_) {
return false;
}
},
},
};
</script>
Expand All @@ -99,4 +161,10 @@ export default {
.v-popper {
display: inline;
}

.popover-error-message {
color: #f00;
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

[nitpick] The hardcoded color value #f00 is a magic number. Consider using a CSS custom property or a semantic color variable from your design system for better maintainability.

Suggested change
color: #f00;
color: var(--popover-error-color, #f00);

Copilot uses AI. Check for mistakes.
font-size: 1.0rem;
margin-top: 0.25rem;
}
</style>