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

Make StatusWidget tools dropdown extensible #3189

Conversation

rafaucau
Copy link
Contributor

@rafaucau rafaucau commented Dec 7, 2021

Changes proposed in this pull request:

This PR allows you to more easily modify the tools dropdown in the StatusWidget component using ItemList.

Until now, if you wanted to add something to this dropdown, you had to push a child:

 extend(StatusWidget.prototype, 'items', (items) => {
    const tools = items.get('tools');
    tools.children.push(<Button>Test</Button>);
  });

If you want to remove / replace a given item it gets harder.
Now such code is enough:

 extend(StatusWidget.prototype, 'toolsItems', (items) => {
    items.add('test', <Button>Test</Button>);
  });

Reviewers should focus on:

Confirm that everything works.

Screenshot

image

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

@rafaucau rafaucau force-pushed the StatusWidget-extendable-tools-dropdown branch from 9b47c90 to d217796 Compare December 7, 2021 00:57
Copy link
Member

@davwheat davwheat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! Just some minor comments from me.

I know @askvortsov1 doesn't want many changes to the Admin dashboard at the moment, since it'll be getting a rewrite in the near future, so I'd appreciate his input.

Personally, I think we can make changes up until v1.2. That way, when rewriting, we can use the existing dashboard as a benchmark to ensure features or extensibility aren't accidentally dropped between the two, and to make the overall migration easier.

js/dist-typings/admin/components/StatusWidget.d.ts Outdated Show resolved Hide resolved
js/src/admin/components/StatusWidget.js Show resolved Hide resolved
@davwheat
Copy link
Member

davwheat commented Dec 7, 2021

Also it looks like you've rebased this incorrectly, as there are some changes to the file versioner since I began writing my review. Could you make sure those are fixed, too?

@rafaucau rafaucau reopened this Dec 7, 2021
@rafaucau
Copy link
Contributor Author

rafaucau commented Dec 7, 2021

Also it looks like you've rebased this incorrectly, as there are some changes to the file versioner since I began writing my review. Could you make sure those are fixed, too?

@davwheat Uhhh, managed to fix my commits. Sorry for that 🤦‍♂️

Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might change things eventually, but for now I'm glad for this to be more extensible.

@askvortsov1 askvortsov1 merged commit 0f2824e into flarum:master Dec 7, 2021
@askvortsov1 askvortsov1 added this to the 1.2 milestone Dec 7, 2021
@rafaucau rafaucau deleted the StatusWidget-extendable-tools-dropdown branch December 7, 2021 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants