pnp / sp-dev-fx-controls-react

Reusable React controls for SPFx solutions
https://pnp.github.io/sp-dev-fx-controls-react/
MIT License
382 stars 379 forks source link

IconPicker does not work in ListViewCommandSet #1297

Closed EA12 closed 1 year ago

EA12 commented 2 years ago

Category

[ ] Enhancement

[x ] Bug

[ ] Question

Version

Please specify what version of the library you are using: [ 3.10.0 ]

Here are the dependencies from the package.json:

"dependencies": {
    "@microsoft/decorators": "1.13.1",
    "@microsoft/rush-stack-compiler-3.2": "^0.10.47",
    "@microsoft/sp-application-base": "1.13.1",
    "@microsoft/sp-core-library": "1.13.1",
    "@microsoft/sp-dialog": "1.13.1",
    "@microsoft/sp-listview-extensibility": "1.13.1",
    "@pnp/common": "^2.11.0",
    "@pnp/logging": "^2.11.0",
    "@pnp/odata": "^2.11.0",
    "@pnp/sp": "^2.11.0",
    "@pnp/spfx-controls-react": "3.10.0",
    "node-sass": "^4.14.0",
    "office-ui-fabric-react": "^7.180.3"
  }

Expected / Desired Behavior / Question

The IconPicker should open up when clicking the Button of the CommandSet.

Observed Behavior

From my console output I can see that the render method of my component with the IconPicker gets called, but the dialog does not come up.

image

Steps to Reproduce

Create a SPFx Extension of type ListViewCommandSet Create code for OnExcecute like this:

  @override
  public onExecute(event: IListViewCommandSetExecuteEventParameters): void {
    if(event.itemId === CommandNames.AssignSvg) {
      const selectedRow: RowAccessor = event.selectedRows[0];
      const id: string = selectedRow.getValueByName('ID');      
      const div = document.createElement('div');

      console.log("Open CContainer.");
      const element: React.ReactElement<IPickerDialogContentProps> = React.createElement(
        CContainer,
        {
          listItemID: Number(id),
          message: "Icon auswählen",
          save: this._save
        }
      );
      ReactDOM.render(element, div);      
    }
    else {
      throw new Error(`Unknown command ${event.itemId}`);
    }
  }

Add a class to hold an IconPicker:

import * as React from "react";
import IPickerDialogContentProps from "./IPickerDialogContentProps";
import { IconPicker } from '@pnp/spfx-controls-react/lib/IconPicker';

export default class CContainer extends React.Component<IPickerDialogContentProps, {}> {
    constructor(props: IPickerDialogContentProps) {
      super(props);
    }

    public render(): React.ReactElement<IPickerDialogContentProps> {
     console.log("render entring in CContainer.");
    return (<IconPicker
    renderOption="dialog"
    onSave={(value) => { this.props.save(value, this.props.listItemID); }}
    buttonLabel={ this.props.message }>
     </IconPicker>);      
    }
  }

My props interface looks like this:

export default interface IPickerDialogContentProps {
    message: string;
    listItemID: number;
    save: (iconName: string, listItemID: number) => void;
  }

If I add a Panel instead of the IconPicker, my code is working:

public render(): React.ReactElement<IPickerDialogContentProps> {
      console.log("render entring in CContainer.");
      return ( <Panel isOpen={true}>
        <h2>This is a custom panel with your own content</h2>
        <TextField value="Hello World" label="Item title" placeholder="Choose the new title" />
        <DialogFooter>
            <DefaultButton text="Cancel" />
            <PrimaryButton text="Save" />
        </DialogFooter>
    </Panel>);      
    }

image

Please note: The IconPicker is calling initializeIcons(), but this method has been already called by the AllItems.aspx of my list (as far as I know). The console warning: Redefining what an icon is may have unintended consequences.

So it looks like the IconPicker cannot be used in a CommandSet, because of a bug.

ghost commented 2 years ago

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

github-actions[bot] commented 2 years ago

Thank you for submitting your first issue to this project.

EA12 commented 2 years ago

Nobody?

michaelmaillot commented 1 year ago

Hi @EA12,

The "problem" with this control is that the icons list relies on a ready-to-use button and nothing else and there's no prop to handle the panel / dialog display. But according to me, it's not a bug.

Even if this could be an improvment idea (to have a new prop to display the icons list without a button), below a workaround that could fit your specific need for a Listview Command Set, using React ref.

Pay attention that the example is using an "any" ref, in order to trigger a private onClick event of the control. But this is not a recommanded approach (maybe you found another workaround, it's been almost a year).

// ...
public onExecute(event: IListViewCommandSetExecuteEventParameters): void {
    console.log(event);
    switch (event.itemId) {
      case 'COMMAND_1':
        this._renderDialogContainer(true);
        break;
      default:
        throw new Error('Unknown command');
    }
  }

private _renderDialogContainer(isDialogDisplayed: boolean): void {
    const element: React.ReactElement<unknown> = React.createElement(
      IconPickerDialog,
      {
        context: this.context,
        displayDialog: isDialogDisplayed,
        closeDialog: this._closeDialogContainer
      }
    );

    ReactDom.render(element, this.dialogContainer);
  }

  private _closeDialogContainer = (): void => {
    this._renderDialogContainer(false);
  }
import * as React from 'react';
import { IconPicker } from '@pnp/spfx-controls-react/lib/controls/iconPicker/IconPicker';

export interface IIconPickerDialogProps {
    displayDialog: boolean;
    closeDialog: () => void;
}

export default class IconPickerDialog extends React.Component<IIconPickerDialogProps, {}> {

    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    private myRef: any;

    constructor(props: IIconPickerDialogProps) {
        super(props);

        this.myRef = React.createRef();
    }

    public componentDidMount(): void {
        this.myRef.current.iconPickerOnClick();
    }

    public componentDidUpdate(prevProps: Readonly<IIconPickerDialogProps>, prevState: Readonly<{}>, snapshot?: any): void {
        if (prevProps.displayDialog !== this.props.displayDialog && this.props.displayDialog) {
            this.myRef.current.iconPickerOnClick();
        }
    }

    public render(): React.ReactElement<IIconPickerDialogProps> {
        return (
            <IconPicker
                ref={this.myRef}
                renderOption='dialog'
                onCancel={() => this.props.closeDialog()}
                onSave={(iconName: string) => { console.log(iconName); this.props.closeDialog() }} />
        );
    }
}

Hope that it helps.

joelfmrodrigues commented 1 year ago

@EA12 is this still an issue?

EA12 commented 1 year ago

@michaelmaillot & @joelfmrodrigues You can close the issue. As I couldn't find a solution, we redesigned the whole thing and there was no need anymore to assign an icon. - Thanks for your effort.