Implement container that gives additional information on the profile setting

[parttimenerd]

This implements a container giving more information than already provided in the "Profile Info" container (see #4209). This gives us the possibility to add more information that is related to the whole profile without cluttering the "Profile Info" container.

This adds a new "Additional Information" button:

When you click on it, you see a floating container with additional information:

The information is specified in the new extra field in the meta section of the profile:

export type ExtraProfileInfoSection = {|
  // section label
  label: string,
  entries: {|
    label: string,
    format: MarkerFormatType,
    // any value valid for the formatter
    value: any,
  |}[],
|};
...
  extra?: ExtraProfileInfoSection[],
...

Sample profile

Deployment Preview

[codecov]

Codecov Report

Base: 88.44% // Head: 88.48% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (feb442b) compared to base (14d46cc).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head feb442b differs from pull request most recent head b0211d0. Consider uploading reports for the commit b0211d0 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4222      +/-   ##
==========================================
+ Coverage   88.44%   88.48%   +0.03%     
==========================================
  Files         282      282              
  Lines       24858    24850       -8     
  Branches     6660     6656       -4     
==========================================
+ Hits        21986    21988       +2     
+ Misses       2667     2660       -7     
+ Partials      205      202       -3     

Impacted Files	Coverage Δ
src/components/app/MenuButtons/MetaInfo.js	85.41% <100.00%> (+2.70%)	⬆️
src/selectors/profile.js	96.21% <100.00%> (+0.01%)	⬆️
src/profile-logic/symbol-store.js	92.98% <0.00%> (-0.24%)	⬇️
src/components/shared/SourceView-codemirror.js	90.81% <0.00%> (-0.10%)	⬇️
src/profile-logic/symbolication.js	86.91% <0.00%> (-0.09%)	⬇️
src/profile-logic/processed-profile-versioning.js	85.21% <0.00%> (-0.07%)	⬇️
src/profile-logic/merge-compare.js	92.74% <0.00%> (-0.03%)	⬇️
src/test/fixtures/profiles/processed-profile.js	94.48% <0.00%> (-0.01%)	⬇️
src/profile-logic/data-structures.js	95.45% <0.00%> (ø)
src/profile-logic/mozilla-symbolication-api.js	13.09% <0.00%> (+1.39%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[canova]

For the profile data format:
I still have doubts about using MarkerFormatType on other places but maybe in the future we can make this more generic across profile. So I'm not opposed to it.

For the UI:
Is there a need for a new overlaying popup exactly? I'm surprised by this because I was always thinking that we can use the same popup and never though of another poup. Adding another popup brings accesibility issues like, how to close, how to go back to previous popup and confusion of opening another overlay from an overlay etc.

I was thinking about expanding the same "profile info" popup with additional information when user clicks on this button. There are <details> and <summary> elements you can use for this as well. This also removes a lot of extra css and code from the PR.
What do you think?

[parttimenerd]

I'm surprised by this because I was always thinking that we can use the same popup and never though of another poup.

Which popup? The current "Profile info" popup is pretty narrow and small. The popup can be opened and closed the same way as the "Profile info" popup.

I would like to present lots of information in this popup. Like all currently running processes on the host system, all environment variables (might have fairly long names), all used libraries.

The CSS (and code) is mostly copied from the existing "Profile info" popup.

[canova]

Do you have an example where this width is not enough for your use case? Since you are moving the value to a new line, you will have more space horizontally. In your current example, it looks like they would all fit.

The problem with the new popup is that it's not obvious that which element it is tied to. For the profile info popup it's obvious that it's tied to "Profile Info" button:

As you can see, there is a clear arrow and the button is in selected state.

But opening a popup from another popup doesn't seem right UX wise.
Also your new section seems more like a modal (especially considering that it dims the outside area). A modal should have a proper close buttons and properly handle the keyboard/focus state for accessibility.
We have a modal currently for keyboard shortcuts, you can it if you press ?:

In this one there is a clear close button, clear title, and the focus state is handled properly.

I also don't think we should use the same modal for this case though.
Lastly I don't think we should add another panel that 99% of our users won't see. This will make future development prone to errors and we'll have to maintain it in the future. I feel like the simplest solution of expanding the current panel is a better compromise for both of us.

[parttimenerd]

Lastly I don't think we should add another panel that 99% of our users won't see. This will make future development prone to errors and we'll have to maintain it in the future. I feel like the simplest solution of expanding the current panel is a better compromise for both of us.

You're probably right.

I currently don't have a case at hand where expanding the current popup to the bottom, and adding scrollbars would not help.

Another idea: Could we extend the "Profile info" to the right? This would alleviate all my width concerns.

[canova]

Another idea: Could we extend the "Profile info" to the right? This would alleviate all my width concerns.

I'm not opposed to it as long as it doesn't make our other side by side key-value items hard to read. Could be good to get feedback from others too.

[parttimenerd]

[julienw]

Thanks for the patch, and for your patience!
I left a few comments, tell me what you think.

First, an optional quick note: we could use a <details> element, which supports this behavior natively and would make it possible to remove the showsMoreInfo state. I'm not sure about the stylability, but it would be good to try using it.

About the formatting abilities, ideally I'd prefer that we extract the formatting mechanisms to another file before we move forward with this PR (and probably also the other one). But if that's too much work it's also good to me that we move forward with this solution and we clean up later. Please tell me your thoughts!

Also it would be good to have a test that exercize this new code. There's a block about the MetaInfo panel in src/test/components/MenuButtons.test.js that you can insert a test to.

Please squash your 2 existing commits and add your review fixes to an extra commit for an easier second review :-)

[parttimenerd]

First, an optional quick note: we could use a

Details

element, which supports this behavior natively and would make it possible to remove the showsMoreInfo state. I'm not sure about the stylability, but it would be good to try using it.

I want to still change the name of the button. So I'll still want to use the state. I could achieve this by setting CSS attributes to the localized strings, but I think this would be hard to understand.

About the formatting abilities, ideally I'd prefer that we extract the formatting mechanisms to another file before we move forward with this PR (and probably also the other one). But if that's too much work it's also good to me that we move forward with this solution and we clean up later. Please tell me your thoughts!

I'd rather like to have it in a later PR (submitted directly after merging this PR and #4201), as it would require many rebases and refactoring is not really the purpose of this PR.

[julienw]

I'd rather like to have it in a later PR (submitted directly after merging this PR and #4201), as it would require many rebases and refactoring is not really the purpose of this PR.

You shouldn't be afraid of rebases :-)

1. Do the refactoring in another PR.
2. Rebase this patch on top of the other PR.
3. Change import and calls to formatters.

My understanding is that this would be about the same amount of work.

That said I'm OK with your approach.

[julienw]

I pushed some changes, can you please have a quick look ? :) thanks

[parttimenerd]

Your changes look good to me.

[parttimenerd]

Short objection: Why does it show "Hide" and not "Show Less"? I find the latter better.

[julienw]

looks good with my changes (that I discussed with Johannes as well)

outdated