Skip to content

Symbol links should respect the formatting provided in overridingTitleInlineContent #645

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Symbol links should respect the formatting provided in overridingTitleInlineContent #645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion src/components/ContentNode.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,10 +430,12 @@ function renderNode(createElement, references) {
const titleInlineContent = node.overridingTitleInlineContent
|| reference.titleInlineContent;
const titlePlainText = node.overridingTitle || reference.title;
const kind = node.overridingTitleInlineContent
? node.overridingTitleInlineContent[0].type : reference.kind;
return createElement(Reference, {
props: {
url: reference.url,
kind: reference.kind,
kind,
role: reference.role,
isActive: node.isActive,
ideTitle: reference.ideTitle,
Expand Down
2 changes: 1 addition & 1 deletion src/components/ContentNode/Reference.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default {
return name !== notFoundRouteName;
},
isSymbolReference() {
return this.kind === 'symbol'
return (this.kind === 'symbol' || this.kind === 'codeVoice')
Copy link
Contributor

@mportiz08 mportiz08 May 8, 2023

Choose a reason for hiding this comment

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

I'm not sure that this is the right approach since it mixes the already well defined values that kind can be with our own custom values like this.

In this specific situation, you'll actually end up with a doubly nested <code> element, which is not ideal, although it may look right in your example. One comes from the reference component that this logic determines and another would would come through the content itself.

I think what we actually want is just to short-circuit this logic in the case that we have a reference with an overridingTitleInlineContent value, which is really close to the approach you have here, but not quite the same. Basically, in the case where a developer has provided custom text, we want to defer to their inline formatting and not apply any extra.

Copy link
Contributor

@mportiz08 mportiz08 May 8, 2023

Choose a reason for hiding this comment

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

I went ahead and opened an alternative PR based on this feedback if you'd like to review that: #647

&& (this.role === TopicRole.symbol || this.role === TopicRole.dictionarySymbol);
},
isDisplaySymbol({ isSymbolReference, titleStyle, ideTitle }) {
Expand Down
30 changes: 30 additions & 0 deletions tests/unit/components/ContentNode.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1254,13 +1254,43 @@ describe('ContentNode', () => {
const reference = wrapper.find('.content').find(Reference);
expect(reference.exists()).toBe(true);
expect(reference.props('url')).toBe('/foo/bar');
expect(reference.props('kind')).toBe('codeVoice');

const codeVoice = reference.find(CodeVoice);
expect(codeVoice.exists()).toBe(true);
expect(codeVoice.text()).not.toBe('FooBar with Code Voice');
expect(codeVoice.text()).toBe('Foo the Bar with Code Voice');
});

it('renders a `Reference` with an overwritten title of type="text"', () => {
const wrapper = mountWithItem({
type: 'reference',
identifier: 'foobar',
overridingTitle: 'Foo with Text',
overridingTitleInlineContent: [
{
type: 'text',
text: 'Foo the Bar with Text',
},
],
}, {
foobar: {
title: 'FooBar',
url: '/foo/bar',
kind: 'bla',
},
});

const reference = wrapper.find('.content').find(Reference);
expect(reference.exists()).toBe(true);
expect(reference.props('kind')).toBe('text');

const codeVoice = reference.find(CodeVoice);
expect(codeVoice.exists()).toBe(false);
expect(reference.text()).not.toBe('FooBar with Code Voice');
expect(reference.text()).toBe('Foo the Bar with Text');
});

it('renders a reference as a none link when isActive is false', () => {
const wrapper = mountWithItem({
type: 'reference',
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/components/ContentNode/Reference.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,22 @@ describe('Reference', () => {
expect(ref.props('url')).toBe('/documentation/uikit/uiview');
});

it('renders a `ReferenceInternalSymbol` for external "codeVoice" kind references', () => {
const wrapper = shallowMount(Reference, {
localVue,
router,
propsData: {
url: '/documentation/uikit/uiview',
kind: 'codeVoice',
role: TopicRole.symbol,
},
slots: { default: 'UIView' },
});
const ref = wrapper.find(ReferenceInternalSymbol);
expect(ref.exists()).toBe(true);
expect(ref.props('url')).toBe('/documentation/uikit/uiview');
});

it('renders a `ReferenceInternal` for external "symbol" kind references with human readable name', () => {
const wrapper = shallowMount(Reference, {
localVue,
Expand Down