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.

Sign up for GitHub

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

Jump to bottom

Replacing empty tables with empty state message #2259

Merged
merged 1 commit into from
Oct 17, 2017
Merged

Replacing empty tables with empty state message #2259

merged 1 commit into from
Oct 17, 2017

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

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

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

@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 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

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

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

@rhamilto
Copy link
Member Author

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

/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

/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

/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

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

@dtaylor113 dtaylor113 dtaylor113 left review comments

@spadgett spadgett Awaiting requested review from spadgett

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