-
Notifications
You must be signed in to change notification settings - Fork 405
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
build: extracted UI components to ui-kit in plugin-base-frontend #3470
Conversation
8b40b31
to
2cea11c
Compare
2cea11c
to
f3358eb
Compare
e93a272
to
6f84eb3
Compare
31ae203
to
0d26102
Compare
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.
LGTM
09863bf
to
303049d
Compare
return records.map((record) => { | ||
return { | ||
...record, | ||
template: <SearchResultRow searchResult={record} />, | ||
} as Omit<OptionWithSearchResult, 'name'> & { name: unknown } as OptionWithHighlightComponent; | ||
}); | ||
}; |
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.
some typing fixes that aren't pretty but it's better then ignoring
_showPopup(){ | ||
if(this.props.shouldShowPopup){ |
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.
this turns out to be unused
const onChangeSelected = useCallback( | ||
(selectItem: SelectItem<unknown> | null) => { | ||
if (!selectItem) { | ||
return; | ||
} | ||
const maybeOption: Option = selectItem as Option; | ||
window.location.replace(`${(window as IWindow).pathToRoot}${maybeOption.location}?query=${maybeOption.name}`); | ||
onSelected(maybeOption); | ||
}, |
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.
another hack just to proceed with legacy code
}; | ||
})(); | ||
|
||
export function initTabs() { |
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.
This is pretty much extracted from dokka-subprojects/plugin-base/src/main/resources/dokka/scripts/platform-content-handler.js
4b131b1
to
c884402
Compare
c884402
to
f4c8702
Compare
f4c8702
to
16bd619
Compare
**/node_modules | ||
**/build |
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.
Belated nitpick: Are these excludes necessary?
build/
directories are already excluded on line 30, and node_modules/
is excluded specifically in dokka-subprojects/plugin-base-frontend/.gitignore
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.
thanks, it's really unnecessary, removed it in https://github.com/Kotlin/dokka/pull/3714/files
Fixes:
#3460
#3461
#3462
The UI component is just a couple of js and css files which are conntected to the markup by bem, if possible
The assets (ts and scss) are compiled by webpack, the results are stored in plugin-base as plain files js and css (both minified and non-minified for the sake of possible customization by users) files and committed to the repo.