Use image listing from registry-image-widgets #1136

stefwalter · 2017-01-18T19:56:03Z

This shares code with the Registry Console and Openshift Web Console for the ImageStream image listing.

stefwalter · 2017-01-18T19:56:41Z

Openshift Web Console View:

stefwalter · 2017-01-18T19:57:06Z

Existing Registry Console Views:

stefwalter · 2017-01-18T19:57:28Z

CC @andreasn @jwforres

stefwalter · 2017-01-18T19:57:41Z

CC @dperpeet

spadgett · 2017-01-18T19:57:45Z

Very cool

jwforres · 2017-01-18T21:55:54Z

I see this is pulling in gettext now @stefwalter is this going to cause the image stream bits to be translated when the rest of the console is not? or does something have to be enabled for gettext to use a different language other than the default

stefwalter · 2017-01-19T07:48:56Z

I see this is pulling in gettext now @stefwalter is this going to cause the image stream bits to be translated when the rest of the console is not? or does something have to be enabled for gettext to use a different language other than the default.

It won't. The because project does not initiallize angular-gettext ... nor does the project load any translations.

jwforres · 2017-01-19T13:32:33Z

OK sounds good, I will pull this down today and try it out

…

On Jan 19, 2017 2:49 AM, "Stef Walter" ***@***.***> wrote: I see this is pulling in gettext now @stefwalter <https://github.com/stefwalter> is this going to cause the image stream bits to be translated when the rest of the console is not? or does something have to be enabled for gettext to use a different language other than the default. It won't. The because project does not initiallize angular-gettext ... nor does the project load any translations. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1136 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABZk7WEMyYkGIvOTvcaF8sAhqScOeHIvks5rTxVpgaJpZM4LnUmW> .

jwforres · 2017-01-19T14:59:26Z

When no tags have been pushed it says "No image streams are present" seems like that should say "No tags are present"

When no tags have been pushed the Image count is empty instead of saying '0'. Also noticed it is inconsistent on this screen whether the labels have a ':' after them. The rest of the console does have ':' after any labels.

This version of the image widgets has some kind of issue with the image layers widget, when i revert back to master and use the image widgets version that points to this same image shows up just fine in the layers widget:

jwforres · 2017-01-19T15:00:58Z

When I hover over the dropdown arrow it messes up the row highlighting, part of the row is gray and part of the row is blue.

jwforres · 2017-01-19T15:03:10Z

The "pushed from" in the originates from column is not on the same horizontal as the tag and the SHA, which creates a strange white gap, seems like it should move up.

jwforres · 2017-01-19T15:03:56Z

The text "To push to an image to this image stream:" isnt right, should be "To push an image to this image stream:"

jwforres · 2017-01-19T15:10:43Z

What is expanding a tag supposed to do? I thought maybe it was supposed to show the history for that tag, but when I expand it then it isn't showing me anything different, even when I know a tag has had multiple images pushed to it. It just seems to add a line between the sha and the timestamp.

jwforres · 2017-01-19T15:13:09Z

The sha causes problems at various responsive sizes, pushing content off screen. It needs to have word-break on it

jwforres · 2017-01-19T15:14:10Z

I'm actually seeing similar issues on the image widget page with long identifiers pushing off the page at mobile

jwforres · 2017-01-19T15:16:05Z

When no images are pushed yet, the table doesn't extend all the way to the right of the screen, as soon as there as an image it does

andreasn · 2017-01-20T12:43:06Z

There is a couple of things I'm not currently satisfied about how this view works, also in the registry.

Tags appearance looks out of place. Maybe this would do well from using the badge look. http://www.patternfly.org/pattern-library/widgets/#badges
The rows are very tall.
The list view looks and works a bit different from both Cockpit and Patternfly (mostly because it's older code from before we refined the appearance of things) http://www.patternfly.org/pattern-library/content-views/list-view/#/expanding-rows
The SHA string is reeeeally long and just breaks the rows apart. This will only get worse with SHA512. I understand the idea about not hiding part of it (opening the question of "will I be able to copy this out?"), but maybe we can ellipsis it a bit and then show the full value on hover.

I'll put together a suggestion in a mockup for how to solve these.

stefwalter · 2017-01-20T13:05:44Z

Thanks for the review @andreasn. May I suggest changes as a follow up pull request, and the first one gets the two projects mostly in sync on the view? Are you okay with that @jwforres

@jwforres and @andreasn: The inline expansion stuff wasn't supposed to show up in the Openshift Web Console. That's a bug.

@andreasn In the registry the entire sha256 is necessary as the docker command line doesn't support abbreviated SHA identifiers for images. But perhaps that's a place where the Openshift Web Console can diverge and the implementation can take a CSS option to truncate this ... just thinking outloud.

andreasn · 2017-01-20T13:34:34Z

I'm OK with doing it as a followup if @jwforres is.

stefwalter · 2017-01-20T16:13:09Z

When no tags have been pushed it says "No image streams are present" seems like that should say "No tags are present"

Fixed.

When no tags have been pushed the Image count is empty instead of saying '0'.

Fixed.

Also noticed it is inconsistent on this screen whether the labels have a ':' after them. The rest of the console does have ':' after any labels.

The Registry Console doesn't use colons, and uses different styling for metadata/data. So to make it consistent I've just added these colons via CSS in the Openshift Web Console use of the widgets.

This version of the image widgets has some kind of issue with the image layers widget, when i revert back to master and use the image widgets version that points to this same image shows up just fine in the layers widget:

Fixed. This was a regression from when we marked is translatable. Now works.

When I hover over the dropdown arrow it messes up the row highlighting, part of the row is gray and part of the row is blue.

This doesn't happen for me. Maybe I tagged the bower registry-image-widgets build incorrectly or something. Should work now. The dropdown arrow is not part of the changes we intended to share with the Openshift Web Console. The Registry Console has a listing pattern that allows expansion of data inline, but that doesn't need to be carried over into the Openshift Web Console.

The "pushed from" in the originates from column is not on the same horizontal as the tag and the SHA, which creates a strange white gap, seems like it should move up.

I'll leave this up to @andreasn's follow up pull request.

The text "To push to an image to this image stream:" isnt right, should be "To push an image to this image stream:"

Fixed.

When no images are pushed yet, the table doesn't extend all the way to the right of the screen, as soon as there as an image it does

Fixed.

The sha causes problems at various responsive sizes, pushing content off screen. It needs to have word-break on it

Added word-break.

stefwalter · 2017-01-20T16:13:40Z

Everything fixed except for the row redesign, which should be done as a follow up, so this feature can get "in" before the feature freeze.

pweil- · 2017-02-03T13:06:18Z

@stefwalter please rebase

@jwforres @spadgett bump for another review since a lot of things have been fixed 👍

stefwalter · 2017-02-06T10:35:33Z

Rebased.

stefwalter · 2017-02-06T10:36:11Z

However origin-web-console no longer authenticates my version of openshift, and I'm running out of time to sort that all out, so I've been unable to do any smoke test after the rebase.

[root@f1 ~]# openshift version
openshift v1.4.1+3f9807a
kubernetes v1.4.0+776c994
etcd 3.1.0-rc.0

petervo · 2017-06-12T21:08:05Z

@jwforres, Thanks, rebased and updated this one.

jwforres · 2017-06-12T21:36:07Z

thanks @petervo

I'm seeing two bugs currently

When you hover over a row the visual style is a little weird where the blue hover color appears on three sides of the tag but not on the left. This one is lower severity and I'd be ok fixing it in a follow-up as long as we get an issue open to track it.
Bad overflow can occur at mobile res in a couple places. The table can overflow, and the docker pull commands can overflow.

jwforres · 2017-06-12T21:46:42Z

app/views/browse/imagestream.html

-                  </tr>
-                </tbody>
-              </table>
+              <registry-imagestream-body imagestream="imageStream">


sorry I didn't notice it before, but the switch to use registry-imagestream-body changes the behavior of how annotations are presented to the user, everywhere else in the UI we do not show these to the user unless they click a link to reveal them. This is because annotations are typically intended for machine use and not end user facing.

petervo · 2017-06-12T21:57:01Z

@jwforres, for the docker commands do you want ellipsis or line wrapping with small sizes. Line wraping looks weird but is better for copy/paste. Is that ok?

jwforres · 2017-06-12T21:57:20Z

app/views/browse/imagestream.html

-                  <dt>Docker pull spec:</dt>
-                  <dd>{{imageStream.status.dockerImageRepository}}</dd>
-                </dl>
-                <annotations annotations="imageStream.metadata.annotations"></annotations>


for reference, this is the annotations directive we use today that handles the show/hide behavior plus how we format annotations when visible

jwforres · 2017-06-12T22:01:52Z

agree wrapping is probably better. i know we do multi-line code style in other places and managed to make it look alright, one example is in _pod-template.html L94, i suspect the "command" class was probably adding some special multi-line stuff

…

On Mon, Jun 12, 2017 at 5:57 PM, petervo ***@***.***> wrote: @jwforres <https://github.com/jwforres>, for the docker commands do you want ellipsis or line wrapping with small sizes. Line wraping looks weird but is better for copy/paste. Is that ok? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1136 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABZk7U3uoWlKRAw6WjXoUaUubDCOmo_zks5sDbQygaJpZM4LnUmW> .

This starts work to share code between the Registry Console and Openshift Web Console for the imagestream page. In particular this commit changes the body and push sections of the page.

This shares code with the Registry Console and Openshift Web Console for the ImageStream image listing.

In the Registry, they use styling rather than colons to differentiate metadata from data. However we can simply add colons via CSS to make these parts consistent with the rest of the Openshift Web Console.

petervo · 2017-06-13T09:33:21Z

@jwforres, opened openshift/registry-image-widgets#8 and built this with that so you can check this out first before merging the other one.

Once that is in i can do a rebuild

jwforres · 2017-06-13T13:19:38Z

looks like the table still has an issue at iphone 5 res which is 320px width

the quick fix is in imagestream.html to wrap <registry-imagestream-body> in a <div class="table-responsive" style="padding: 0 5px;">

this will prevent the layout like the nav from getting messed up when the table overflows the page, it wraps the table in a scrollable content area with a border when at mobile resolutions

if you are fine just making that change in this PR then we can merge the PR to image-registry-widgets and cut that release, let me know

petervo · 2017-06-13T14:10:57Z

@jwforres pushed an update that fixes it for me.

jwforres · 2017-06-13T14:35:07Z

@petervo I have your latest changes but I can still reproduce it with a tag name that is long

petervo · 2017-06-13T15:10:24Z

@jwforres updated again.

jwforres · 2017-06-13T15:38:36Z

thanks LGTM now, think we can merge the other PR

jwforres · 2017-06-13T15:39:07Z

I'll LGTM the other PR and you can merge/release it if you are ready

petervo · 2017-06-13T15:40:57Z

@jwforres, i don't have access to marge or deploy to npm. I'll see if can I track down someone who does.

jwforres · 2017-06-13T15:46:13Z

@petervo ah k i can merge and create the tag for bower, i dont think i have the npm access but bower is all we need to the console

jwforres · 2017-06-13T15:47:00Z

@petervo ok 0.0.9 should be available for bower now

jwforres · 2017-06-13T16:08:08Z

[merge]

openshift-bot · 2017-06-13T16:15:18Z

Evaluated for origin web console merge up to 3ac0f90

openshift-bot · 2017-06-13T16:15:26Z

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1511/) (Base Commit: 9c3ecd1)

stefwalter force-pushed the image-stream-from-registry branch from 20e21d0 to 0dbefb1 Compare January 18, 2017 20:36

stefwalter force-pushed the image-stream-from-registry branch from 0dbefb1 to e3cb5cc Compare January 20, 2017 16:22

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2017

pweil- assigned jwforres Feb 3, 2017

stefwalter force-pushed the image-stream-from-registry branch from e3cb5cc to da7ab4a Compare February 6, 2017 10:34

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 6, 2017

jwforres reviewed Jun 12, 2017

View reviewed changes

petervo and others added 3 commits June 13, 2017 02:28

Use ImageStream 'body' and 'push' from registry-image-widgets

3229c99

This starts work to share code between the Registry Console and Openshift Web Console for the imagestream page. In particular this commit changes the body and push sections of the page.

Use image listing from registry-image-widgets

58f7482

This shares code with the Registry Console and Openshift Web Console for the ImageStream image listing.

Add colons after <dt> tags that come from the Registry

4aa6b71

In the Registry, they use styling rather than colons to differentiate metadata from data. However we can simply add colons via CSS to make these parts consistent with the rest of the Openshift Web Console.

petervo force-pushed the image-stream-from-registry branch from e89a66b to 13ac6a9 Compare June 13, 2017 09:32

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2017

petervo force-pushed the image-stream-from-registry branch from 13ac6a9 to b4bf383 Compare June 13, 2017 14:10

petervo force-pushed the image-stream-from-registry branch from b4bf383 to 7f36fa2 Compare June 13, 2017 15:09

Overide registryAnnotations directive to match the annotations directive

3ac0f90

petervo force-pushed the image-stream-from-registry branch from 7f36fa2 to 3ac0f90 Compare June 13, 2017 15:52

jwforres approved these changes Jun 13, 2017

View reviewed changes

openshift-bot merged commit cf4b122 into openshift:master Jun 13, 2017

Use image listing from registry-image-widgets #1136

Use image listing from registry-image-widgets #1136

Conversation

stefwalter commented Jan 18, 2017

stefwalter commented Jan 18, 2017

stefwalter commented Jan 18, 2017

stefwalter commented Jan 18, 2017

stefwalter commented Jan 18, 2017

spadgett commented Jan 18, 2017

jwforres commented Jan 18, 2017

stefwalter commented Jan 19, 2017

jwforres commented Jan 19, 2017 via email

jwforres commented Jan 19, 2017

jwforres commented Jan 19, 2017

jwforres commented Jan 19, 2017

jwforres commented Jan 19, 2017

jwforres commented Jan 19, 2017

jwforres commented Jan 19, 2017

jwforres commented Jan 19, 2017

jwforres commented Jan 19, 2017

andreasn commented Jan 20, 2017

stefwalter commented Jan 20, 2017

andreasn commented Jan 20, 2017

stefwalter commented Jan 20, 2017

stefwalter commented Jan 20, 2017 • edited Loading

pweil- commented Feb 3, 2017

stefwalter commented Feb 6, 2017

stefwalter commented Feb 6, 2017 • edited Loading

petervo commented Jun 12, 2017

jwforres commented Jun 12, 2017

jwforres Jun 12, 2017

Choose a reason for hiding this comment

petervo commented Jun 12, 2017

jwforres Jun 12, 2017

Choose a reason for hiding this comment

jwforres commented Jun 12, 2017 via email

petervo commented Jun 13, 2017

jwforres commented Jun 13, 2017

petervo commented Jun 13, 2017

jwforres commented Jun 13, 2017

petervo commented Jun 13, 2017

jwforres commented Jun 13, 2017

jwforres commented Jun 13, 2017

petervo commented Jun 13, 2017

jwforres commented Jun 13, 2017

jwforres commented Jun 13, 2017

jwforres commented Jun 13, 2017

openshift-bot commented Jun 13, 2017

openshift-bot commented Jun 13, 2017 • edited Loading

stefwalter commented Jan 20, 2017 •

edited

Loading

stefwalter commented Feb 6, 2017 •

edited

Loading

openshift-bot commented Jun 13, 2017 •

edited

Loading