Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

Pinging @elastic/kibana-app (Team:KibanaApp)

Thanks for working on this @lykims . I checked the PR and it seems like the empty header is still adding margin to the top - this space should be used by the chart:

Also there are now unused css classes lnsWorkspacePanelWrapper__pageContentHeader and lnsWorkspacePanelWrapper__pageContentHeader--unsaved

Hi @flash1293!

Good observation! If the CSS classes lnsWorkspacePanelWrapper__pageContentHeader and lnsWorkspacePanelWrapper__pageContentHeader--unsaved are removed, the margin would still be there because of the UI element EuiPageContentHeader that has the CSS class euiPageContentHeader with margin-bottom: 24px;.

If the UI element EuiPageContentHeader is removed, then the logic !emptyExpression || title wouldn't be needed because the h1 elements would be redundant:

<EuiPageContent className="lnsWorkspacePanelWrapper">
  {!emptyExpression || title ? (
    <EuiScreenReaderOnly>
      <h1 data-test-subj="lns_ChartTitle">
        {title ||
          i18n.translate('xpack.lens.chartTitle.unsaved', { defaultMessage: 'Unsaved' })}
      </h1>
    </EuiScreenReaderOnly>
  ) : (
    <EuiScreenReaderOnly>
      <h1 data-test-subj="lns_ChartTitle">
        {title ||
          i18n.translate('xpack.lens.chartTitle.unsaved', { defaultMessage: 'Unsaved' })}
      </h1>
    </EuiScreenReaderOnly>
  )}
  ...

Then, the workspace part could simply look like:

<EuiPageContent className="lnsWorkspacePanelWrapper">
  <EuiScreenReaderOnly>
    <h1 data-test-subj="lns_ChartTitle">
      {title ||
        i18n.translate('xpack.lens.chartTitle.unsaved', { defaultMessage: 'Unsaved' })}
    </h1>
  </EuiScreenReaderOnly>
  ...

and the prop emptyExpression would become unused.

What do you think?

@lykims Sounds good to me!

If the CSS classes lnsWorkspacePanelWrapper__pageContentHeader and lnsWorkspacePanelWrapper__pageContentHeader--unsaved are removed, the margin would still be there because of the UI element EuiPageContentHeader that has the CSS class euiPageContentHeader

Yeah, removing the classes and removing the margin are two separate things I just put in a single comment

@flash1293 I pushed new changes to be reviewed.

Thanks @lykims , I will take a look later today 👍

@elasticmachine merge upstream

Jenkins, test this

@lykims Seems like there's a little additonal cleanup to do:

ERROR x-pack failed
x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.test.tsx:39:9 - error TS2322: Type '{ children: Element; dispatch: Mock<any, any>; framePublicAPI: Mocked; visualizationState: {}; visualizationId: string; visualizationMap: { ...; }; datasourceMap: {}; datasourceStates: {}; emptyExpression: boolean; }' is not assignable to type 'IntrinsicAttributes & WorkspacePanelWrapperProps'.
Property 'emptyExpression' does not exist on type 'IntrinsicAttributes & WorkspacePanelWrapperProps'.

@elasticmachine merge upstream

Jenkins, test this.

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: ac9dccc

Metrics [docs]

async chunks size

History

• 💔 Build #85822 failed d01c15b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream