Improved secret style

[andresmgot]

Description of the change

Follow up of #1882 (review)

Applicable issues

• ref Update Kubeapps styles/classes to use clarity.design #1354

[andresmgot]

Note that I couldn't use the Input component, the style was not properly applied :/

[Angelmmiguel]

Np. I think this is a special case of Input usage

[andresmgot]

I needed this because by default, readOnly input fields don't have the border-bottom but I wanted it to show the length of the input field.

[Angelmmiguel]

Interesting!

[Angelmmiguel]

LGTM, although please check my comments before landing it :)

[Angelmmiguel]

Interesting!

[Angelmmiguel]

Np. I think this is a special case of Input usage

[Angelmmiguel]

This code may cause an error if the user navigates to a different page when the tooltip is showed. This component will be unmounted and the setCopied method will disappear from the context. To avoid this, you can use useRef to keep the timeout reference and useEffect to control it:

Suggested change

	const [copied, setCopied] = React.useState(false);
	const toggleDisplay = () => setHidden(!hidden);
	const setCopiedTrue = () => {
	setCopied(true);
	setTimeout(() => {
	setCopied(false);
	}, 1000);
	};
	const [copied, setCopied] = React.useState(false);
	const toggleDisplay = () => setHidden(!hidden);
	const copyTimeout = useRef(null);
	// You can now remove this intermediate method.
	const setCopiedTrue = () => {
	setCopied(true);
	};
	useEffect(() => {
	if (copied) {
	copyTimeout.current = setTimeout(() => {
	setCopied(false);
	}, 1000);
	}
	return () => {
	if (copyTimeout.current != null) {
	clearTimeout(copyTimeout.current);
	}
	}
	}, [copied])

[andresmgot]

good catch!

[Angelmmiguel]

I'd change this to a label and connect it to the input

[Angelmmiguel]

Add a aria-label to this icon so it notifies screen readers its purpose:

Suggested change

	<CdsIcon shape="copy-to-clipboard" size="md" solid={true} />
	<CdsIcon shape="copy-to-clipboard" size="md" solid={true} aria-label={`Copy ${name} secret value to the clipboard`}/>

[absoludity]

+1, looking excellent. Just coming back to this discussion from the previous PR:

The goal of this first row is to try to show everything needed to access the repo: readiness, access URL and credentials:

Not all the credentials are actually needed, I don't think? For eg., in the example screenshot, it's only the WP secret which is relevant and needed. I wonder if it'd be worth differentiating.

Yes, they are not. In other cases it will be even less relevant for secrets like encoded certs but we have no way of knowing which of them are "important" to build that list.

Perhaps we could have a way - if a schema has been included for a chart (for easy deployment), it would tend to include secrets that are relevant to users, I'd think? So we could do something like: if the chart has a schema and the schema includes some secret fields, highlight those credentials only (and otherwise highlight all or none - they're still in the resources below). Not suggesting for this PR.

[andresmgot]

Perhaps we could have a way - if a schema has been included for a chart (for easy deployment), it would tend to include secrets that are relevant to users, I'd think? So we could do something like: if the chart has a schema and the schema includes some secret fields, highlight those credentials only (and otherwise highlight all or none - they're still in the resources below). Not suggesting for this PR.

The schema uses the values.yaml, there is no way to map .values.wordpressPassword to the secret containing those credentials.

Ideally, what app developers could do is tagging secrets with an annotation. Something like kubeapps.com/credentials/username: foo, kubeapps.com/credentials/password: wordpress-password (being wordpress-password the key of the secret data). That way we could ensure those are the app credentials and we can highlight only that.