Package Dialog layout: two columns

[fiskus]

I moved Intercom widget to the left side because it conflicts with the submit button.

[codecov-io]

Codecov Report

Merging #2001 (c771928) into master (dd6ad48) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master    #2001   +/-   ##
=======================================
  Coverage   46.47%   46.47%           
=======================================
  Files         332      332           
  Lines       16532    16532           
  Branches     2231     2208   -23     
=======================================
  Hits         7684     7684           
  Misses       7938     7938           
  Partials      910      910           

Flag	Coverage Δ
api-python	89.35% <ø> (ø)
catalog	7.32% <0.00%> (ø)
lambda	92.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
catalog/app/components/JsonEditor/AddRow.js	0.00% <ø> (ø)
catalog/app/components/JsonEditor/JsonEditor.js	0.00% <ø> (ø)
catalog/app/components/JsonEditor/Row.js	0.00% <ø> (ø)
catalog/app/containers/Bucket/PackageCopyDialog.js	0.00% <ø> (ø)
...talog/app/containers/Bucket/PackageCreateDialog.js	0.00% <0.00%> (ø)
...p/containers/Bucket/PackageDialog/PackageDialog.js	0.00% <ø> (ø)
...og/app/containers/Bucket/PackageDialog/Skeleton.js	0.00% <ø> (ø)
...talog/app/containers/Bucket/PackageUpdateDialog.js	0.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd6ad48...c771928. Read the comment docs.

[nl0]

I moved Intercom widget to the left side because it conflicts with the submit button.

@fiskus maybe just hide it underneath the dialog? (use smaller z-index) bc footer layout takes intercom button placement into account

[nl0]

The code looks good, but i think we should use more conventional layout for fullscreen dialogs with an appbar on the top with title and buttons (see MUI example). This will also solve the problem with the intercom widget.

Also some comments on the 2-column layout:

• i think we need more spacing between the columns (it should be larger than spacing between section title and contents)

• it would be nice to add a title to the first section (tho i'm not sure which one to use) and align the sections better

[fiskus]

i think we should use more conventional layout for fullscreen dialogs with an appbar on the top with title and buttons (see MUI example).

I don't think it's conventional for desktop, it's more natural for mobile.

---

Note that sections horizontally aligned now. Files bottom is the metadata bottom, when they are empty. FIles bottom is the workflows bottom when they are in maximum height, and workflow is empty. They can't be pixel perfect aligned, because workflows input can have additional description, and because empty Select has less height than filled [1] (I tried to fix it at our side, but I think additional code doesn't worth it)

[1] mui/material-ui#21587

[fiskus]

just hide it underneath the dialog? (use smaller z-index)

It seems that intercom doesn't have API for this. I can add body .intercom-lightweight-app to globalStyles.js

[fiskus]

• I've added more space between columns. Unfortunately <M.grid container /> and <M.Dialog /> are not frields, there vertical scroll, I used my own styles.
• Added "Main" subheader.
• Made right column vertically flexible:

[nl0]

looks good, definitely better than the previous version, tho i'd explore a material-style fullscreen dialog-based approach too (in another PR ofc).

I don't think it's conventional for desktop, it's more natural for mobile.

it seems like it's quite conventional for material design framework.

anyways, feel free to merge

[fiskus]

<M.Dialog fullscreen /> is where I started

Also, Material Design spec has a note, that fullscreen dialog is mobile only: https://material.io/components/dialogs#full-screen-dialog

[nl0]

<M.Dialog fullscreen /> is where I started

Also, Material Design spec has a note, that fullscreen dialog is mobile only: https://material.io/components/dialogs#full-screen-dialog

ok i see