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

Limit columns added to strategies panel table #247

Merged
merged 4 commits into from
Oct 14, 2022
Merged

Conversation

jernestmyers
Copy link
Member

@jernestmyers jernestmyers commented Sep 26, 2022

Resolves #246

Follow up work that improves the UI of the modal will be handled when replacing the existing WDKClient CheckboxTree with the new version from CoreUI, as outlined in #248.

@jernestmyers jernestmyers marked this pull request as ready for review September 28, 2022 16:21
@jernestmyers
Copy link
Member Author

Please note that I changed the tooltip verbiage to read

Please select no more than 80 columns

In our scrum , "Please select fewer than 80 columns" was suggested, but since 80 columns is allowable, I tweaked the verbiage accordingly.

Copy link
Member

@dmfalke dmfalke left a comment

Choose a reason for hiding this comment

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

This looks great, save for the minor formatting issue. Once that is addressed, I think this is good to go.

Comment on lines 87 to 101
const updateButton = (
<button
type="button"
className="btn"
disabled={areMaxColumnsExceeded ? true : false}
onClick={() => {
if (columnsDialogSelection) {
requestColumnsChoiceUpdate(columnsDialogSelection, question.urlSegment)
}
showHideAddColumnsDialog(false);
}}
>
Update Columns
</button>
);
Copy link
Member

Choose a reason for hiding this comment

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

Indentation here is a little off.

@jernestmyers jernestmyers merged commit 59b8aab into master Oct 14, 2022
@jernestmyers jernestmyers deleted the fix/limit-columns branch October 14, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"select all" issue - limit columns in strategy panel workspace
2 participants