Skip to content

docs(ui5-shellbar): adjust samples to UXC guidelines #11340

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dobrinyonkov
Copy link
Contributor

@dobrinyonkov dobrinyonkov commented Apr 16, 2025

Updated the Shellbar samples to follow UXC guidelines. Removed APIs that will be deprecated.

@dobrinyonkov dobrinyonkov changed the title docs(ui5-shellbar): adjust samples to UXC docs(ui5-shellbar): adjust profile area sample to UXC guidelines Apr 16, 2025
@dobrinyonkov dobrinyonkov force-pushed the shellbar-samples-uxc-adjust branch from a8b3d5e to 8874d89 Compare April 16, 2025 06:13
@dobrinyonkov dobrinyonkov changed the title docs(ui5-shellbar): adjust profile area sample to UXC guidelines docs(ui5-shellbar): adjust samples to UXC guidelines Apr 28, 2025
@dobrinyonkov dobrinyonkov requested a review from Copilot May 7, 2025 08:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the ShellBar samples to align with the new UXC guidelines by replacing deprecated APIs and improving the sample components.

  • Updated basic samples to use the new ui5-shellbar-search component for search functionality.
  • Removed deprecated attributes and components in advanced samples and replaced them with the new user menu components.
  • Updated corresponding main.js files to import new components and handle search scopes accordingly.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
packages/website/docs/_samples/fiori/ShellBar/Basic/sample.html Replaced ui5-input with ui5-shellbar-search to update the search field API.
packages/website/docs/_samples/fiori/ShellBar/Basic/main.js Imported ui5-shellbar-search to support the updated basic sample.
packages/website/docs/_samples/fiori/ShellBar/Advanced/sample.html Removed deprecated attributes and updated the search field and user menu integration.
packages/website/docs/_samples/fiori/ShellBar/Advanced/main.js Removed deprecated imports and added new ones, including handling dynamic search scope changes.

<ui5-user-menu-item icon="action-settings" text="Setting" data-id="setting"></ui5-user-menu-item>
<ui5-user-menu-item icon="official-service" text="Legal Information">
<ui5-user-menu-item text="Terms of Use" data-id="terms-of-use"></ui5-user-menu-item>
<ui5-user-menu-item text="Private Policy" data-id="privacy-policy"></ui5-user-menu-item>
Copy link
Preview

Copilot AI May 7, 2025

Choose a reason for hiding this comment

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

Consider renaming "Private Policy" to "Privacy Policy" to align with standard terminology.

Suggested change
<ui5-user-menu-item text="Private Policy" data-id="privacy-policy"></ui5-user-menu-item>
<ui5-user-menu-item text="Privacy Policy" data-id="privacy-policy"></ui5-user-menu-item>

Copilot uses AI. Check for mistakes.

Comment on lines +69 to +71
item.remove();
});
Copy link
Preview

Copilot AI May 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using a more efficient method such as setting searchScope.innerHTML to an empty string if supported, to remove all search items and improve maintainability.

Suggested change
searchScope.items.forEach(item => {
item.remove();
});
searchScope.innerHTML = "";

Copilot uses AI. Check for mistakes.

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