This repository was archived by the owner on Dec 6, 2019. It is now read-only.
Fixed Multisite current site evaluation issue: the site selector does…#465
Open
peternazarov wants to merge 1 commit into
Open
Fixed Multisite current site evaluation issue: the site selector does…#465peternazarov wants to merge 1 commit into
peternazarov wants to merge 1 commit into
Conversation
… not show the current site selection bases on the current Site context.
Contributor
|
@gbxe Thanks for the PR, but not sure I understand the issue being solved here. Is there a bug in Habitat itself that you have reproduction steps for, and if so can you open a related Issue? Or is it a problem encountered when reusing this module in another solution? Thanks! |
Contributor
Author
|
When you switch the site, the current site highlight does not change. I have just completed Helix Workshop in Munich, Germany, where I discovered that issue and fixed it. I think IsCurrent property should not be part of the data model because of its value is Site context dependent. However, there would be more Habitat solution code refactoring required. Please review the fix and evaluate whether it is good enough in your perspective. |
Contributor
Author
|
This is the fix for #466 |
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.

Fixed Multisite current site evaluation issue: the site selector does not show the current site selection based on the current Site context.
Implementation note: maybe SiteDefinition should just have the corresponding extension method instead of "IsCurrent" property. Then the extension method can be unit tested in isolation. However, it would make the coding less intuitive because you will not observe the extension method from IntelliSense unless the extension method class's namespace is exposed to the context.