add toast for import #10290

abhinandan13jan · 2021-10-21T17:42:56Z

Addresses

https://issues.redhat.com/browse/ODC-6392
https://issues.redhat.com/browse/ODC-6380

Screenshot

Tests

Added test suite for importToast

Browser conformance

Chrome

jerolimov · 2021-10-25T10:51:38Z

@abhinandan13jan Good direction, but this solution has currently some UI issues and ignores some import cases:

I believe we should not show the link twice. Is it possible to create a link which is also copyable? Maybe it could look like this:

In ticket ODC-6380 we will need also change this in the topology sidebar. It's definitely not required to reuse this component here, but maybe you can take a look.

I would recommend one link with two icons. :)

This UI change would also help that the link is not rendered without any space behind the "Deployment created successfully" message.

At the moment the link has also no http:// prefix. When the user clicks on the link this opens http://localhost:9000/react-web-app-3-christoph.apps-crc.testing and shows a 404 instead of her/his application.
And finally, the code works currently only for Deployments. It should also work for DeploymentConfigs and Serverless Deployments. Please also notice that the Route checkbox could be unchecked by the user. So if the user creates a Deployment (or DC or Knative Service) without the Route option we should still show a notification but could not display any link in that case.

frontend/packages/dev-console/src/components/import/ImportForm.tsx

frontend/packages/dev-console/src/components/import/toast/ImportToastcontent.tsx

abhinandan13jan · 2021-10-25T15:10:00Z

@jerolimov I've updated accordingly.. Can you please re-visit

abhinandan13jan · 2021-10-26T12:14:34Z

/retest

frontend/packages/dev-console/src/components/import/toast/ImportToastcontent.tsx

frontend/packages/dev-console/src/components/import/__tests__/ImportToastcontent.spec.tsx

jerolimov · 2021-10-27T11:41:07Z

@openshift/team-devconsole-ux @beaumorley I think we need your UX feedback here,

a) Are you fine with the toast / notification layout in the first comment?

Sorry, I personally would still suggest not to use the gray bar to show a link. We use this to copy and paste data, like ConfigMap/Secrets keys or values. Esp. we do not use such a box on the topology sidebar to show the same infos. We show there just a link.

I suggest a UI more like the sidebar:

Wdyt?

b) There are two strings which shows which resources are created

RESOURCE ADDED
Deployment created succeessfully.

This strings currently doesn't depend on the Deployment type we created. Should we add different cases* for that or could we use a more generic term here like "Application created successfully."? Wdyt?

*Deployment vs DeploymentConfig vs Serverless Service.

abhinandan13jan · 2021-10-29T13:55:03Z

@jerolimov Created new component to handle all the discussed cases. Updated screenshots please check

jerolimov · 2021-10-29T13:59:43Z

/assign

serenamarie125 · 2021-10-29T17:10:03Z

don't think we want all CAPS here @abhinandan13jan but will let @beaumorley confirm

serenamarie125 · 2021-10-29T19:37:28Z

my concern with these two icons next to eachother is that the hit target is so small, people trying to copy will likely click the link frequently. @beaumorley what are your thoughts?

abhinandan13jan · 2021-11-02T09:43:44Z

@serenamarie125 @beaumorley
Please let me know if increasing the padding space for the last copy-icon helps or what is to be done here?

Updated heading to Sentence case and handled case sensitive filesystem names
cc: @jerolimov

beaumorley · 2021-11-08T21:29:31Z

Hi all,
I discussed this with Serena today. PatternFly does have a copy link pattern. It is part of copy to clipboard like in the example above but I've formatted it slightly differently and included the external link.

Could you do something like this? There is an option in the PF spec for copy to clipboard in a form and the size of the field is this size.

@serenamarie125 what do you think?

abhinandan13jan · 2021-11-09T15:44:35Z

@beaumorley @jerolimov I've updated the component to look similar to the CopyToClipboard component

Please note that:

The icons are positioned same as that of the CopytoClipboard component of console
This would apply across all areas where RouteLocation is exposed.

beaumorley · 2021-11-09T15:55:32Z

Looks great. Can you just add a period after "Deployment created successfully." <-- Like that.

Thank you.

christianvogt · 2021-11-09T16:00:36Z

@beaumorley
This seems have gone through several iterations already. But I'd like to share my opinion. As a user I've been accustomed to knowing that blue text on websites equates to a link. I've also been accustomed to knowing that any link can be easily copied via right click.

While having a copy to clipboard button may be useful. my typical go to is still to right-click and copy link address. Furthermore this design for a link goes against the norm that links are blue. Having the https schema is also important in identifying text as a link. Especially if we're trying to convey to the user that this is their service URL.

serenamarie125 · 2021-11-09T16:11:58Z

@beaumorley I see what @christianvogt is saying ... here's my 2 cents

New component pluses

I like the spacing of this new component much better
I like the feedback / tooltip on the copy icon

Concerns

I was concerned about the look of the code component being odd here and possibly confusing
I think the External Link icon would be before Copy because I think that's more frequently used
with this component, the URL is no longer a link, you'd have to click the icon. Maybe this is an issue because it's not consistent with how it's been displayed forever

jerolimov · 2021-11-09T16:46:17Z

+1 for showing a blue link, but also just my opinion! And to be fair, blue on gray doesn't look good. 😕

At the end someone needs to decide and we (as group) should agree one a UI now before Abhi updates it again and again. If we can not do here we should to that on PM/Eng/UX Sync...

¯_(ツ)_/¯

beaumorley · 2021-11-09T16:59:33Z

Agree with your feedback @christianvogt . And your +1 Christoph. It was my initial reaction but then I tried to work with the requirement and PF patterns. I think we agree that blue linked text conveys that this is a link. The external icon conveys it is taking user to new place. These are exiting conventions.

The copy icon is being used unconventionally. @serenamarie125 do you want to go back to the other design and just put more space between the icons? Alternatively, we could just not add the copy link at all.

Here is the alter with more space between the icons.

@serenamarie125 is this what you had originally suggested?

openshift-bot · 2021-11-19T02:07:31Z

/retest-required