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

Fixed the logic to check for SideKickButton in the Core db #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maheshjwagh
Copy link
Contributor

Fixed the logic which was preventing the SideKick button being shown in the Desktop menu.
Returning if SitecoreSideKick button is null.

@@ -69,7 +69,7 @@ public static void EnsureDesktopButton()
return;

Item sk = core.GetItem(new ID(SidekickButton));
if (sk != null)
if (sk == null)
Copy link
Owner

Choose a reason for hiding this comment

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

This change would make it so if the sidekick button doesn't exist yet it breaks out of the method thus not creating the button. Are you tracking a particular bug related to this?

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 wasn't seeing the SideKick button on the Desktop right menu. I checked and the Core db had all the items refrenced by the fields of this class, which led me to this change, after which button appeared.

Copy link
Owner

Choose a reason for hiding this comment

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

So, the logic here is that if the button definition already exists then return out of the method. If sk is not null, that means it's already defined and we can skip the rest of the check. The change you have here makes it so a fresh install of sidekick would skip adding the button if the button doesn't exist.

You can confirm this by trying it on a fresh Sitecore install from SIM

@markgibbons25
Copy link

I think this is resolved now, haven't had this issue on the latest version, but I did on an older version.

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.

3 participants