Skip to content

Replacing empty tables with empty state message#2259

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
rhamilto:issue-109
Oct 17, 2017
Merged

Replacing empty tables with empty state message#2259
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
rhamilto:issue-109

Conversation

@rhamilto
Copy link
Member

@rhamilto rhamilto commented Oct 12, 2017 •
edited
Loading

Fixes #109
Replaces #497

HACK DAY!

Before:
127 0 0 1-8443-console-project-my-project-dc-browse-deployments
127 0 0 1-8443-console-project-my-project-dc-browse-deployments 1

After:
localhost-9000-dev-console-project-my-project-empty-quota
localhost-9000-dev-console-project-my-project-delete-browse-config-maps-labelfilter 7b 22matchexpressions 22- 5b 7b 22key 22- 22foo 22 22operator 22- 22exists 22 22values 22- 5b 5d 7d 5d 7d

@rhamilto rhamilto requested a review from spadgett October 12, 2017 20:48
@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 12, 2017
dtaylor113
dtaylor113 reviewed Oct 12, 2017
View reviewed changes
app/views/browse/config-maps.html
</tr>
</tbody>
</table>
<div ng-if="(configMaps | hashSize) == 0">
Copy link
Contributor

@dtaylor113 dtaylor113 Oct 12, 2017

Choose a reason for hiding this comment

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

Hi, would be great to use the pfEmptyState component.

Copy link
Member Author

@rhamilto rhamilto Oct 13, 2017

Choose a reason for hiding this comment

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

@dtaylor113, I opted to use the pre-existing empty-state styles that pre-date pfEmptyState that were already in use in the console for the sake of consistency.

@spadgett
Copy link
Member

spadgett commented Oct 13, 2017

@rhamilto I don't see the $scope.filterWithZeroResults value being reset to false in most components. Is that a problem?

@rhamilto
Copy link
Member Author

rhamilto commented Oct 13, 2017

@rhamilto I don't see the $scope.filterWithZeroResults value being reset to false in most components. Is that a problem?

It did not appear to be in my testing. Is there a better way?

@benjaminapetersen
Copy link
Contributor

benjaminapetersen commented Oct 13, 2017

Nice, def visually is an improvement.

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 16, 2017
@rhamilto rhamilto changed the title Replacing empty tables with empty state message [WIP] Replacing empty tables with empty state message Oct 16, 2017
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 16, 2017
@rhamilto rhamilto changed the title [WIP] Replacing empty tables with empty state message Replacing empty tables with empty state message Oct 17, 2017
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 17, 2017
@rhamilto
Copy link
Member Author

rhamilto commented Oct 17, 2017

Revised to replace filter alert with empty state message containing "Clear Filter" link and addressing comment from @spadgett. PTAnotherL.

@rhamilto
Copy link
Member Author

rhamilto commented Oct 17, 2017

Filter button is labeled "Clear filters". Empty state link is labeled "Clear Filter" as that's what we're using on the overview. These should be consistent?

@spadgett
Copy link
Member

spadgett commented Oct 17, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2017
@spadgett spadgett mentioned this pull request Oct 17, 2017
Closed
@rhamilto
Copy link
Member Author

rhamilto commented Oct 17, 2017

/retest

@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2017
@spadgett
Copy link
Member

spadgett commented Oct 17, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2017
@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2017
@rhamilto rhamilto added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2017
@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Oct 17, 2017

Automatic merge from submit-queue.

@openshift-merge-robot openshift-merge-robot merged commit 075822b into openshift:master Oct 17, 2017
@rhamilto rhamilto deleted the issue-109 branch October 17, 2017 18:49
@rhamilto rhamilto mentioned this pull request Oct 23, 2017
Merged
openshift-merge-robot added a commit that referenced this pull request Oct 23, 2017
@openshift-merge-robot
Merge pull request #2341 from rhamilto/storage-filter-empty-state
3d44d1c 
Automatic merge from submit-queue.

Update storage filter empty state to new standard

I overlooked the filtered empty state message for storage in #2259.  This fixes that.

![screen shot 2017-10-23 at 11 05 38 am](https://user-images.githubusercontent.com/895728/31896715-765d8ffa-b7e2-11e7-9776-e9c5d36cc222.PNG)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spadgett spadgett Awaiting requested review from spadgett

1 more reviewer

@dtaylor113 dtaylor113 dtaylor113 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@spadgett spadgett

Labels

lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rhamilto @spadgett @benjaminapetersen @openshift-merge-robot @dtaylor113 @openshift-ci-robot