Issue 168 - add key prop to ssh keys and certs

[mreynolds389]

Fixes console warning: Each child in a list should have a unique "key" prop

Signed-off-by: Mark Reynolds mreynolds@redhat.com

Fixes: #168

[carma12]

I tried the code and it seems that the error is still there. As the changes look good, not sure if this could be related to my local instance. Could you please confirm that the code is working on your side too?

[mreynolds389]

I tried the code and it seems that the error is still there. As the changes look good, not sure if this could be related to my local instance. Could you please confirm that the code is working on your side too?

Hmmm seems to work for me. If I revert the code the console warning comes back

[carma12]

I tried the code and it seems that the error is still there. As the changes look good, not sure if this could be related to my local instance. Could you please confirm that the code is working on your side too?

Hmmm seems to work for me. If I revert the code the console warning comes back

That's odd - Could you please rebase the branch with the last changes and try again? Otherwise, maybe there is something that has been cached somewhere...

[mreynolds389]

I tried the code and it seems that the error is still there. As the changes look good, not sure if this could be related to my local instance. Could you please confirm that the code is working on your side too?

Hmmm seems to work for me. If I revert the code the console warning comes back

That's odd - Could you please rebase the branch with the last changes and try again? Otherwise, maybe there is something that has been cached somewhere...

Rebased, rebuilt, etc. I confirmed again that it is working correctly. Now I can't add a cert (I guess that's not implemented yet?), so I can only test with SSH keys.

[carma12]

I think I found the problem - Apparently, a key element was needed in the buildTableBody function. After adding it, the console warning is gone.

const buildTableBody = (elements: DictWithName[]) => {
    return (
      <div className="pf-v5-u-display-table">
        {elements.map((element) => {
          return (
            <div key={"table-body-" + element.key}>   // <--- Fragment has been replaced by a `div` element, containing the `key`
              <div className="pf-v5-u-display-table-row">
                <div className="pf-v5-u-display-table-cell">
                  <p className="pf-v5-u-mb-xs pf-v5-u-mr-xs pf-v5-u-font-weight-bold">
                    {element.key + ": "}
                  </p>
                </div>
                <div className="pf-v5-u-display-table-cell">
                  <p
                    className="pf-v5-u-mb-xs"
                    id={element.name}
                    data-name={element.name}
                  >
                    {element.value}
                  </p>
                </div>
              </div>
            </div>  // <--- Closing `div` instead of Fragment
          );
        })}
      </div>
    );
  };

I am still unsure why I was getting that warning from my side (most probably a cache thing). In any case, both code modifications should fix the problem :)

[mreynolds389]

I am still unsure why I was getting that warning from my side (most probably a cache thing). In any case, both code modifications should fix the problem :)

Yeah I'm confused how you need this for it work for you, when it works fine for me. I tried different browsers, etc, it's not a caching thing on my end. Anyway I'll add this code ...

[mreynolds389]

I am still unsure why I was getting that warning from my side (most probably a cache thing). In any case, both code modifications should fix the problem :)

Yeah I'm confused how you need this for it work for you, when it works fine for me. I tried different browsers, etc, it's not a caching thing on my end. Anyway I'll add this code ...

Ah this was for certificates. I was only testing SSH keys because for me adding certificates is broken or not implemented. Does adding certs in the UI work for you?

[carma12]

I am still unsure why I was getting that warning from my side (most probably a cache thing). In any case, both code modifications should fix the problem :)

Yeah I'm confused how you need this for it work for you, when it works fine for me. I tried different browsers, etc, it's not a caching thing on my end. Anyway I'll add this code ...

Ah this was for certificates. I was only testing SSH keys because for me adding certificates is broken or not implemented. Does adding certs in the UI work for you?

Oh, I see. Sorry for not specifying which field :). 'SSH public keys' works fine. The 'Certificates' field had that error warning whose solution I provided above in this thread.

The addition of certificates works fine for me if I add it directly from the 'Settings' page. As this PR hasn't been merged yet, the addition of new Certificates should not work from the kebab yet.

[carma12]

Changes LGTM.