doc(FullScreenButton): add FullScreenOption documentation #6005
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new FullScreenButton component and enhances fullscreen functionality by adding a Selector property to FullScreenOption, which enables targeting HTML elements using CSS selectors. It also updates related documentation, localization files, and samples while deprecating the ButtonIcon property in favor of Icon.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTest/Services/FullScreenServiceTest.cs | Updated test for FullScreenOption to include the Selector property |
| src/BootstrapBlazor/wwwroot/modules/fullscreen.js | Added support for using the Selector property with querySelector |
| src/BootstrapBlazor/Components/FullScreen/FullScreenOption.cs | Introduced a new nullable string property, Selector |
| src/BootstrapBlazor/Components/FullScreen/FullScreenButton.razor.cs | Updated documentation and deprecated ButtonIcon in favor of Icon |
| src/BootstrapBlazor.Server/docs.json | Added documentation entry for fullscreen-button |
| Locales (zh-CN.json, en-US.json) | Updated localization entries for FullScreenButton and related descriptions |
| src/BootstrapBlazor.Server/Extensions/MenusLocalizerExtensions.cs | Added a new menu item for FullScreenButton |
| Samples (FullScreens.razor, FullScreenButtons.razor, FullScreenButtons.razor.cs) | Updated samples to reflect component usage |
Contributor
Reviewer's GuideThis pull request introduces a new UI component for toggling fullscreen mode, implemented with Razor and C# logic. This component is customizable with parameters for icons, target elements, and display text. The underlying fullscreen functionality was enhanced by enabling elements to be targeted via CSS selectors; this involved modifying the options class and updating the corresponding client-side JavaScript. To support these additions, new demonstration pages were created, existing samples were refactored, localization resources for English and Chinese were updated, and the component was integrated into the documentation and navigation systems. Unit tests were also updated to cover the new targeting capabilities. Finally, an older property in the new component was marked as deprecated in favor of a newer parameter. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
- The PR title could be updated to better reflect the introduction of the
FullScreenButtoncomponent and the newSelectortargeting option. - Consider adding a
TargetSelectorparameter toFullScreenButtonto align its targeting capabilities with the newSelectoroption inFullScreenOption.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+9
to
+11
| else if (options.selector) { | ||
| el = document.querySelector(options.selector); | ||
| } |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Added CSS selector based lookup for full-screen elements.
Validate options.selector to prevent querySelector errors and unexpected behavior.
Suggested change
| else if (options.selector) { | |
| el = document.querySelector(options.selector); | |
| } | |
| else if (typeof options.selector === "string" && options.selector.trim() !== "") { | |
| try { | |
| el = document.querySelector(options.selector); | |
| } catch (e) { | |
| console.warn("Invalid CSS selector provided:", options.selector, e); | |
| el = null; | |
| } | |
| } |
| var option = new FullScreenOption() { Element = new("test01", null), Id = "test", Selector = "test-selector" }; | ||
| Assert.NotNull(option.Id); | ||
| Assert.Null(option.Element.Context); | ||
| Assert.Null(option.Selector); |
Contributor
There was a problem hiding this comment.
issue (testing): Incorrect assertion for Selector property.
You initialized Selector to "test-selector", so Assert.Null(option.Selector) will always fail. Replace it with Assert.Equal("test-selector", option.Selector) (or at least Assert.NotNull(option.Selector)) to verify the property was set correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #6002
Summary By Copilot
This pull request introduces a new
FullScreenButtoncomponent, enhances the existing fullscreen functionality, and updates localization and documentation files to support these changes. The most important changes are grouped below by theme.New
FullScreenButtonComponent:FullScreenButtoncomponent inFullScreenButtons.razorto allow users to make the entire webpage or specific elements fullscreen. Includes localized titles, introductions, and attributes for customization such asIcon,FullScreenExitIcon,TargetId, andText. ([[1]](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-1e3ebfd2f457daaa68b4e5f84ac9f2f4d4fd21d4cb541d1f2fad1cd56b5e6fe4R1-R18),[[2]](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-91b94eea8e9f31b131d3fdbe168e7bd4b4a19681b1d5dc2c6496309a5e5bbe79R1-R52))Enhancements to Fullscreen Functionality:
FullScreenOptionclass to include a newSelectorproperty, allowing fullscreen functionality to target elements using CSS selectors. ([src/BootstrapBlazor/Components/FullScreen/FullScreenOption.csR22-R26](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-4ddee33c988bae6aa9b9f6fa21648d78337994a41a95f7ff1f08c4164846b849R22-R26))fullscreen.jsto support theSelectorproperty for targeting elements viaquerySelector. ([src/BootstrapBlazor/wwwroot/modules/fullscreen.jsR9-R11](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-8aa429d27b934de4f85adf96dcb3d72eb24fe159e093ab6101bf0dbd3aa8bc67R9-R11))FullScreenServiceTest.csto validate the newSelectorproperty inFullScreenOption. ([test/UnitTest/Services/FullScreenServiceTest.csL91-R94](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-8834010fd624e2564d81a021541aa98a8b2b57d4b74efcf65a7a9e2348ef07c8L91-R94))Localization Updates:
FullScreenButtonand updated existing entries inen-US.jsonandzh-CN.jsonto reflect the new component and its functionality. ([[1]](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-790d42ebea731775f0fee88a92b20fadb044e53706fd6d3025dfa095df9b1b41L2254-R2265),[[2]](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-790d42ebea731775f0fee88a92b20fadb044e53706fd6d3025dfa095df9b1b41L4926-R4931),[[3]](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-746b8cc3c80dd10d0a1bf77b8d6571652e15bcc7c33dcac0954d93b1a728cc7fL2254-R2265),[[4]](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-746b8cc3c80dd10d0a1bf77b8d6571652e15bcc7c33dcac0954d93b1a728cc7fL4926-R4931))Documentation and Navigation:
FullScreenButtoninMenusLocalizerExtensions.csto integrate it into the demo navigation. ([src/BootstrapBlazor.Server/Extensions/MenusLocalizerExtensions.csR1185-R1190](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-0da175523aa74d67c07cc125ac55664960a8e14e9b980fc8394ee1b9fff866f7R1185-R1190))docs.jsonto include thefullscreen-buttonentry for documentation purposes. ([src/BootstrapBlazor.Server/docs.jsonL235-R236](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-7f22699a321cdff3a7bd4a3f85ae66d0fbe78560628329c39cca784d9d64dfb7L235-R236))Minor Adjustments:
ButtonIconproperty inFullScreenButton.razor.csin favor of the newIconparameter. ([src/BootstrapBlazor/Components/FullScreen/FullScreenButton.razor.csL14-R14](https://github.com/dotnetcore/BootstrapBlazor/pull/6005/files#diff-f182532ae1eb08180641260b2b44480261d072f72fb663989cfc129e296d962aL14-R14))Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enhance fullscreen functionality by introducing a new FullScreenButton component and extending FullScreenOption with CSS selector support
New Features:
Enhancements:
Documentation:
Tests: