Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Toolbar: add 'kebabLimit' option. #162

Merged
merged 4 commits into from
Dec 11, 2019

Conversation

martinpovolny
Copy link
Member

@martinpovolny martinpovolny commented Dec 10, 2019

Added option to configure the "kebabization" of the menu items.

  • kebabLimit === -1 means no compacting
  • kebabLimit === 0 means always compact into kebab
  • other values of kebabLimit give number of items to keep, before the rest is kebabized.

TODO

  • update specs
  • add specs for the new behavior

Testing

Running the npm run server gives you an example of behavior for different values of kebabLimit.

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1683703

customb-2019-12-11_08-58

Further changes:

  • update tests to make them pass (unrelated),
  • make count optional the same as kebabLimit (based on PR review feedback)

@martinpovolny martinpovolny added bug Something isn't working wip labels Dec 10, 2019
@martinpovolny martinpovolny changed the title Toolbar: add 'kebabLimit' option. [WIP] Toolbar: add 'kebabLimit' option. Dec 10, 2019
@martinpovolny martinpovolny added enhancement New feature or request and removed bug Something isn't working labels Dec 10, 2019
Added option to configure the "kebabization" of the menu items.

 * kebabLimit === -1 means no compacting
 * kebabLimit === 0 means always compact into kebab
 * other values of give number of items to keep, before the rest is kebabized.
@martinpovolny martinpovolny changed the title [WIP] Toolbar: add 'kebabLimit' option. Toolbar: add 'kebabLimit' option. Dec 11, 2019
@martinpovolny martinpovolny removed the wip label Dec 11, 2019
@Hyperkid123
Copy link
Contributor

@martinpovolny can you update the snapshot please?

@@ -10,12 +10,46 @@ const wrapperComponent = () => (
<React.Fragment>
<div className="toolbar-pf row">
<Toolbar
count={1}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can I ask what does the count does and why is it required?

Copy link
Member Author

Choose a reason for hiding this comment

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

In manageiq the toolbar needs count of selected items in the GTL on the page.

Anyway: making it optional, adding default value.

@miq-bot
Copy link
Member

miq-bot commented Dec 11, 2019

Checked commits martinpovolny/react-ui-components@8e1ef18~...a0d656b with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@@ -91,17 +107,23 @@ export const Toolbar = props => (
.filter(toolbarGroupHasContent)
.map((group, index) =>
/* eslint react/no-array-index-key: "off" */
<ToolbarGroup key={index} onClick={props.onClick} group={collapseCustomGroups(group)} />)
<ToolbarGroup key={index} onClick={props.onClick} group={collapseCustomGroups(group, props.kebabLimit)} />)
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose we don't have anything better than index right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not for the group. But the index is stable.

Copy link
Contributor

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

Looks good.

@mzazrivec mzazrivec merged commit 082e477 into ManageIQ:master Dec 11, 2019
@mzazrivec mzazrivec self-assigned this Dec 11, 2019
@martinpovolny
Copy link
Member Author

🎉 This PR is included in version 0.11.57 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants