Merged
Conversation
tdonohue
approved these changes
Jun 21, 2019
Member
tdonohue
left a comment
tdonohue
left a comment
There was a problem hiding this comment.
👍 Thanks for the correction here @LotteHofstede . Looks good to me too
4science-it
pushed a commit
to 4Science/dspace-angular
that referenced
this pull request
May 11, 2023
DSC-854 Approved-by: Davide Negretti
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
I reverted the changes from #424.
This issue was fixed earlier in this commit 49dc4b8#diff-c793158e8f9f65b6e8317b40dcb28bc9
I believe that the SearchService:getSearchLink() method should not return a different path for each page that could possibly contain a search page, like for example the MyDSpace page.
If we were to do this anyway, we would also need this method to return the search links for the journal and person pages, because these also contain search pages; similar to the MyDSpace page.
This is why I refactored this in #398 to make sure the
InPlaceSearchboolean was added to the necessary components, that - when true - will make sure to use the current page as a search page instead of the/searchpage.