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

Fixing Issue 1911 #1922

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Conversation

NishkalankBezawada
Copy link
Contributor

Q A
Bug fix? [x]
New feature? [ ]
New sample? [ ]
Related issues? fixes #1911

What's in this Pull Request?

This PR fixes

  1. [ModernTaxonomyPicker] - Adding a new term adds the term to the parent level, but not to the child level. #1911
  2. SCSS class naming warnings generated during the build as below
css warnings
  1. Updated the .nvmrc file, current .nvmrc file as below
image

Problem

When adding a new term using ModernTaxonomyPicker, the newly added term is adding to the parent directly. Instead it should be added as a child to its parent as below.

Untitled+video+(16)

If the panel is refreshed, the newly added term is properly set unders its parent.

Observation

The method updateTaxonomyTreeViewWithNewTermItems in the TaxonomyTree component is called while adding new terms from ModernTaxonomyPicker control, in which by default, the level of the term where it should be added is set to be the main group, instead, it should be to the parent term.

      const g: IGroup = {
        name: termNames[0]?.name,
        key: term.id,
        startIndex: -1,
        count: 50,
        level: groupToAddTermTo.level + 1, //This should be parent term
        isCollapsed: true,
        data: { skiptoken: "", term: term },
        hasMoreData: term.childrenCount > 0,
      };
      if (g.hasMoreData) {
        g.children = [];
      }
      groupToAddTermTo.children = [...(groupToAddTermTo.children ?? []), g];

Solution

Added a new property as parentTerm which is only used while adding new terms.

    updateTaxonomyTreeViewCallback?: (
      newTermItems?: ITermInfo[],
      parentTerm?: ITermInfo[], //only for adding new terms
      updatedTermItems?: ITermInfo[],
      deletedTermItems?: ITermInfo[]
    ) 

And

  1. Find the find the parent term level
  2. Assign it to the level

As below,

  const updateTaxonomyTreeViewWithNewTermItems = (
    newTermItems: ITermInfo[], parentTerm?: ITermInfo[]
  ): void => {
    for (const term of newTermItems) {
      const findGroupContainingTerm = (currentGroup: IGroup): IGroup => {
        if (!term.parent || currentGroup.key === term.parent.id) {
          return currentGroup;
        }
        if (currentGroup.children?.length > 0) {
          for (const child of currentGroup.children) {
            const foundGroup = findGroupContainingTerm(child);
            if (foundGroup) {
              return foundGroup;
            }
          }
        }
        return null;
      };

     // Find the parent term level
      const findParentTermLevel = (groups: IGroup[], parentTermId: string): number | null => {
        for (const group of groups) {
          if (group.key === parentTermId) {
            return group.level;
          }
          if (group.children && group.children.length > 0) {
            const level = findParentTermLevel(group.children, parentTermId);
            if (level !== null) {
              return level;
            }
          }
        }
        return null;
      };
      
      // get the parent term level
      const parentTermLevel = findParentTermLevel([groups[0]], parentTerm[0].id );
      const groupToAddTermTo = findGroupContainingTerm(groups[0]);
      let termNames = term.labels.filter(
        (termLabel) =>
          termLabel.languageTag === props.languageTag &&
          termLabel.isDefault === true
      );
      if (termNames.length === 0) {
        termNames = term.labels.filter(
          (termLabel) =>
            termLabel.languageTag === props.termStoreInfo.defaultLanguageTag &&
            termLabel.isDefault === true
        );
      }

      const g: IGroup = {
        name: termNames[0]?.name,
        key: term.id,
        startIndex: -1,
        count: 50,
        level: parentTermLevel + 1, // Assign it to the level
        isCollapsed: true,
        data: { skiptoken: "", term: term },
        hasMoreData: term.childrenCount > 0,
      };
      if (g.hasMoreData) {
        g.children = [];
      }
      groupToAddTermTo.children = [...(groupToAddTermTo.children ?? []), g];
      props.setTerms((prevTerms) => {
        const nonExistingTerms = newTermItems.filter((newTerm) =>
          prevTerms.every((prevTerm) => prevTerm.id !== newTerm.id)
        );
        return [...prevTerms, ...nonExistingTerms];
      });
    }
  };

With this change, the newly added terms are adhering to the flow, to add as a child term to the parent (Selected term) as below.

Untitled+video+(21)

Thanks,
Nish

@joaojmendes joaojmendes self-assigned this Dec 12, 2024
@joaojmendes joaojmendes added the status:fixed-next-drop Issue will be fixed in upcoming release. label Dec 12, 2024
@joaojmendes joaojmendes added this to the 3.21.0 milestone Dec 12, 2024
@joaojmendes
Copy link
Collaborator

Hi, @NishkalankBezawada. Thank you for the fix.

@joaojmendes joaojmendes merged commit 3aec762 into pnp:dev Dec 12, 2024
1 check passed
@NishkalankBezawada
Copy link
Contributor Author

Hi, @NishkalankBezawada. Thank you for the fix.

Hey @joaojmendes João, This was very quick.

Thanks for reviewing this,
Nish

@NishkalankBezawada NishkalankBezawada deleted the feature/FixIssue-1911 branch December 12, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:fixed-next-drop Issue will be fixed in upcoming release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants