Skip to content

add full screen#1344

Merged
iceljc merged 1 commit intoSciSharp:masterfrom
iceljc:master
May 4, 2026
Merged

add full screen#1344
iceljc merged 1 commit intoSciSharp:masterfrom
iceljc:master

Conversation

@iceljc
Copy link
Copy Markdown
Collaborator

@iceljc iceljc commented May 4, 2026

No description provided.

@iceljc iceljc merged commit e1de5c5 into SciSharp:master May 4, 2026
0 of 4 checks passed
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add full screen support to embedding data model

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add FullScreen property to EmbeddingData model for full screen control
• Update Membase plugin to enable full screen mode for embedded content
• Configure iframe styling with full width and height dimensions
• Clean up unused imports in Membase plugin
• Fix Membase website URL to use www subdomain
Diagram
flowchart LR
  A["EmbeddingData Model"] -->|"Add FullScreen property"| B["Full Screen Control"]
  C["MembasePlugin"] -->|"Enable full screen"| D["Embedded Content"]
  C -->|"Add styling"| E["iframe Configuration"]
  C -->|"Update URL"| F["www.membase.dev"]
Loading

Grey Divider

File Changes

1. src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs ✨ Enhancement +5/-0

Add FullScreen property to EmbeddingData

• Add new FullScreen boolean property to EmbeddingData class
• Property controls whether embedded content displays in full screen mode
• Includes XML documentation comment explaining the property purpose

src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs


2. src/Plugins/BotSharp.Plugin.Membase/MembasePlugin.cs ✨ Enhancement +4/-4

Enable full screen for Membase embedded content

• Remove unused imports: BotSharp.Abstraction.Graph and BotSharp.Plugin.Membase.Interfaces
• Update Membase favicon URL from https://membase.dev to https://www.membase.dev
• Add HtmlStyle property with iframe dimensions: width: 100%; height: 90%;
• Set FullScreen property to true for the Relationships menu embedding

src/Plugins/BotSharp.Plugin.Membase/MembasePlugin.cs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 4, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. FullScreen always serialized 🐞 Bug ☼ Reliability
Description
EmbeddingData.FullScreen is a non-nullable bool with no JsonIgnore condition, so /plugin/menu
responses will always include fullScreen:false whenever EmbeddingInfo is present and OpenAPI may
mark it as required, altering the API contract for existing clients. This can break strict schema
validation or generated client deserialization that relied on the previous schema.
Code

src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs[R80-83]

+    /// <summary>
+    /// Whether to enable full screen or not
+    /// </summary>
+    public bool FullScreen { get; set; }
Evidence
EmbeddingData uses JsonIgnore(WhenWritingNull) for optional fields, but the newly added
FullScreen has no ignore condition and is non-nullable, so it will serialize its default value
(false) under ASP.NET Core’s default System.Text.Json settings. The /plugin/menu endpoint
returns List<PluginMenuDef> directly, and global JSON options do not configure a default ignore
condition, meaning defaults are not omitted automatically.

src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs[37-84]
src/Infrastructure/BotSharp.OpenAPI/BotSharpOpenApiExtensions.cs[153-161]
src/Infrastructure/BotSharp.OpenAPI/Controllers/Setting/PluginController.cs[22-70]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`EmbeddingData.FullScreen` is a non-nullable `bool` without a JSON ignore condition, so it will be serialized even when not explicitly set (default `false`). This changes the `/plugin/menu` response shape and can cause breakages for strict or generated clients.

### Issue Context
Other `EmbeddingData` fields are treated as optional and omitted when null via `JsonIgnore(WhenWritingNull)`, but `FullScreen` is always present.

### Fix Focus Areas
- src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs[37-84]

### Suggested fix
Pick one of these patterns:
1) **Nullable + omit when null (recommended for backward-compat):**
```csharp
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public bool? FullScreen { get; set; }
```
Then keep `FullScreen = true` where needed.

2) **Non-nullable + omit when default:**
```csharp
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
public bool FullScreen { get; set; }
```
This omits `false` from JSON while keeping a non-nullable API surface in C#.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant