Skip to content

Conversation

@itaigilo
Copy link
Contributor

Change Description

In order to support the extension of object viewers, adding a WebUI plugin for it.

The plugin includes:

  • init function, allowing one-time init by config
  • get function, allowing to provide a custom renderer, by file extension or by the object's content type.

@itaigilo itaigilo added area/UI Improvements or additions to UI exclude-changelog PR description should not be included in next release changelog minor-change Used for PRs that don't require issue attached labels Jul 31, 2025

async commit(repoId, branchId, message, metadata = {}) {
const response = await apiRequest(`/repositories/${repoId}/branches/${branchId}/commits`, {
const response = await apiRequest(`/repositories/${encodeURIComponent(repoId)}/branches/${encodeURIComponent(branchId)}/commits`, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not related, but should be done.


type ConfigType = {
storages: StorageConfigType[] | null;
storages?: StorageConfigType[];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also aligning style, to use the ? style.

};

type VersionConfig = {
upgrade_recommended?: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not related, but used in the code, so added here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I usually avoid file reformatting, but this one was just a little unformatted.

@Annaseli Annaseli requested a review from a team July 31, 2025 14:06
Copy link
Contributor

@guy-har guy-har left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks

const fileType = guessType(props.contentType, props.fileExtension)
const pluginManager = usePluginManager();

const customRenderer = pluginManager.customObjectRenderers.get(props.contentType, props.fileExtension)
Copy link
Contributor

Choose a reason for hiding this comment

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

The signature of get is for contentType and fileExtension is string, while their type is string | null
FWIU you should change the signature or pass undefined in case of null (i.e (props.contentType ?? undefined, props.fileExtension ?? undefined)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, apparently even IntelliJ noted about this...

Updated -
It required some more null/undefined changes in other files...

custom_viewers?: Array<CustomViewerConfig>;
};

type CustomViewerConfig = {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
type CustomViewerConfig = {
type CustomViewer = {

pre_sign_support_ui: boolean;
};

type UIConfigType = {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
type UIConfigType = {
type UIConfig = {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll also align StorageConfigType.

type CustomViewerConfig = {
name: string;
url: string;
content_types?: Array<string>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we sometimes use use snake case and sometimes pascal case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The snake case is what's returned from the API as is,
The pascal case is constructed in the js code.
Since the ConfigType is built in the js (see getConfig()),
I kept it as pascal case.
If it's critical I can align, but this has some internal sense.

Copy link
Contributor Author

@itaigilo itaigilo left a comment

Choose a reason for hiding this comment

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

Thanks @guy-har for the review.

Updated.

pre_sign_support_ui: boolean;
};

type UIConfigType = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll also align StorageConfigType.

type CustomViewerConfig = {
name: string;
url: string;
content_types?: Array<string>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The snake case is what's returned from the API as is,
The pascal case is constructed in the js code.
Since the ConfigType is built in the js (see getConfig()),
I kept it as pascal case.
If it's critical I can align, but this has some internal sense.

const fileType = guessType(props.contentType, props.fileExtension)
const pluginManager = usePluginManager();

const customRenderer = pluginManager.customObjectRenderers.get(props.contentType, props.fileExtension)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, apparently even IntelliJ noted about this...

Updated -
It required some more null/undefined changes in other files...

@itaigilo itaigilo merged commit 53ec766 into master Aug 1, 2025
42 checks passed
@itaigilo itaigilo deleted the feature/custom-object-renderers-plugin-v2 branch August 1, 2025 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/UI Improvements or additions to UI exclude-changelog PR description should not be included in next release changelog minor-change Used for PRs that don't require issue attached

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants