Remove duplicate template areas from the site view sidebar #54942
Conversation
This file contains 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
scruffian
changed the title
|
Size Change: +51 B (0%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
Mamaduka
reviewed
Sep 30, 2023
| @@ -116,14 +116,30 @@ export default function HomeTemplateDetails() { | |||
| * which contains the template icon and fallback labels | |||
| */ | |||
| const templateAreas = useMemo( () => { | |||
| // Keep track of template part IDs that have already been added to the array. | |||
| const templatePartIds = []; | |||
There was a problem hiding this comment.
Suggestion: This could be a Set, it ensures that values are unique and has helpful methods like .has and .add.
There was a problem hiding this comment.
I tried that, but the problem is that the two arrays have clientIds which are different, so set treats them as distinct items. We could create a new array that contains only the properties we need and then use set...
There was a problem hiding this comment.
The templatePart.id isn't clientId; from TT4 examples, duplicates would be post-meta.
Example:
// Keep track of template part IDs that have already been added to the array.
const templatePartIds = new Set();
const filterOutDuplicateTemplateParts = ( currentTemplatePart ) => {
// If the template part has already been added to the array, skip it.
if ( templatePartIds.has( currentTemplatePart.templatePart.id ) ) {
return;
}
// Add to the array of template part IDs.
templatePartIds.add( currentTemplatePart.templatePart.id );
return currentTemplatePart;
};There was a problem hiding this comment.
Oh see what you mean, clever. I was just trying to cast it as a set. thanks, I'll update the PR
|
Question: how can I best test this? |
TT4 has duplicated template areas on it's home page |
Testing instructions:Activate Twenty Twenty-Four. Test results:The PR solves the issue, the correct number of areas are showing for the home template. Aside: |
draganescu
force-pushed
the
fix/duplicate-template-areas
branch
from
7e8c84d to
aeae7b4
Compare
|
@draganescu thanks for the changes, it looks good to me |
justintadlock
added
the
[Feature] Site Editor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
This is another attempt at #54861.
This PR removes duplicate template areas from the sidebar when in Site View.
The list of areas is based on the template parts used on the page, but we may use some template parts twice.
Why?
We only want to display these template areas and corresponding parts once in the sidebar even if they are used multiple times.
How?
By updating
templateAreasinsidepackages/edit-site/src/components/sidebar-navigation-screen-template/home-template-details.jsto remove template parts with the same ID.Screenshot