Skip to content
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

Navbar: fixed bug affecting the search-bar's isOpen prop on initial load #1236

Conversation

akashyeole
Copy link
Member

@akashyeole akashyeole commented May 10, 2024

By creating this pull request you agree to the terms in CONTRIBUTING.md.
https://github.com/Infineon/.github/blob/master/CONTRIBUTING.md
--- DO NOT DELETE ANYTHING ABOVE THIS LINE ---

CONTRIBUTING.md also tells you what to expect in the PR process.

Description
Created a new getNavComponents() function, which returns an object containing the siblings of the search-bar.
Using this function and de-structuring, {rightSideSlot, leftSideSlot, rightAssignedNodes, leftAssignedNodes, navbarProfile, leftMenuItems, rightMenuItems, topRowWrapper} - constants can be extracted. This reduces code repetition.

And using event emission of internalState in search-bar component, to prevent changing state when component is loaded.

Related Issue
#1227

📦 Published PR as canary version: 21.9.1--canary.1236.3ce35e16b7e34b687f6448e294b667380db13343.0

✨ Test out this PR locally via:

npm install @infineon/infineon-design-system-stencil@21.9.1--canary.1236.3ce35e16b7e34b687f6448e294b667380db13343.0
# or 
yarn add @infineon/infineon-design-system-stencil@21.9.1--canary.1236.3ce35e16b7e34b687f6448e294b667380db13343.0

@akashyeole akashyeole added the patch patch version bump label May 10, 2024
@akashyeole akashyeole self-assigned this May 10, 2024
Copy link
Contributor

github-actions bot commented May 10, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-05-28 07:29 UTC

Copy link
Contributor

--STORYBOOK-PREVIEW--

@tishoyanchev
Copy link
Contributor

tishoyanchev commented May 14, 2024

@akashyeole
1 - Please write a proper description
2 - By dividing the code inside handleSearchBarToggle into two separate functions, you are using a lot of repetitive statements. Can you try to outsource some of them into separate re-usable functions so they don't repeat? eg: this.el.shadowRoot.querySelector('.navbar__sidebar-top-row-wrapper') is repeated twice.
3 - Inside componentDidLoad(), you are invking adjustSearchBarWidth(), which checks if isOpen prop is true, and if it is, it invokes the hideNavComponents() func, which changes the searchBarIsOpen State. Changing a State inside componentDidLoad() is usually not recommended, and should be avoided, as the warning error shows in the console log. What you can do instead is emit the internalState value of the Search-bar from inside the search-bar component inside the componentWillLoad(). In that case, you won't need the two extra functioins that you created (adjustSearchBarWidth and getSearchBar)

@tishoyanchev tishoyanchev changed the title Navbar: bug fix where search was not rendered in full width search bar set to open by default Navbar: fixed bug affecting the search-bar's isOpen prop on initial load May 28, 2024
@tishoyanchev tishoyanchev merged commit 6eb104f into master May 28, 2024
8 checks passed
Copy link
Contributor

🚀 PR was released in v21.9.1 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch patch version bump released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navbar: searchBarIsOpen control in Storybook has an outdated functionality
2 participants