-
Notifications
You must be signed in to change notification settings - Fork 125
Move extendedDocLink creation to ModelElementRenderer #2085
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
Conversation
Also separate it from oneLineDoc and update the templates accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this should clean things up.
} | ||
return _oneLineDoc; | ||
} | ||
String get oneLineDoc => _documentation.asOneLiner; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
glad the no-super workaround is gone
test/model_test.dart
Outdated
'Returns a length. <a href="fake/WithGetterAndSetter/lengthX.html">[...]</a>')); | ||
expect(lengthX.oneLineDoc, equals('Returns a length.')); | ||
expect(lengthX.extendedDocLink, | ||
equals(ModelElementRendererHtml().renderExtendedDocLink(lengthX))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should have at least one literal matching string here for the Html renderer test, especially since it is very targeted -- otherwise we're simply validating that extendedDocLink returns whatever the renderer does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@jdkoren Also, whenever I touch the templates I like to upload sample screenshots. It helps make sure there's not some subtle rendering problem I introduced, as the unit tests don't always cover how the browser shows something. Could you upload a sample screenshot or two of pages with the changed templates? |
Also separate it from oneLineDoc and update the templates accordingly.