Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discover] In Context view, Row Height widget is missing #174486

Open
Tracked by #194790
smokris opened this issue Jan 8, 2024 · 6 comments
Open
Tracked by #194790

[Discover] In Context view, Row Height widget is missing #174486

smokris opened this issue Jan 8, 2024 · 6 comments
Labels
enhancement New value added to drive a business result Feature:Discover Discover Application impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort papercut Small "burr" in the product that we should fix. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.

Comments

@smokris
Copy link

smokris commented Jan 8, 2024

Kibana version: 8.11.3

Elasticsearch version: 8.11.3

Server OS version: CentOS Stream 8

Browser version: Firefox 121.0

Browser OS version: macOS 14.1.2

Original install method (e.g. download page, yum, from source, etc.): RPM

Steps to reproduce:

  1. Open Discover
  2. Notice that there's a widget that allows you to change the Row Height (Single, Auto fit, Custom)
  3. Open the "Expanded document" view
  4. Click "Surrounding documents"
  5. Notice that the Row Height widget is missing

Expected behavior: Since the main Discover view's row height carries over into the Context view, I'd expect to be able to see and change the row height from the Context view, too.

Screenshots (if relevant):

Main Discover view, with Row Height widget:
Screenshot 2024-01-08 at 14-53-46 Discover - Elastic

Context view, lacking Row Height widget:
Screenshot 2024-01-08 at 14-48-28 Elastic

Errors in browser console (if relevant): -

Provide logs and/or server output (if relevant): -

@smokris smokris added the bug Fixes for quality problems that affect the customer experience label Jan 8, 2024
@botelastic botelastic bot added the needs-team Issues missing a team label label Jan 8, 2024
@kertal
Copy link
Member

kertal commented Jan 9, 2024

confirmed, thx for reporting

@kertal kertal added Feature:Discover Discover Application Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Jan 9, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 9, 2024
@kertal kertal added enhancement New value added to drive a business result impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort and removed bug Fixes for quality problems that affect the customer experience labels Jan 18, 2024
@kertal
Copy link
Member

kertal commented Jan 18, 2024

this was intentional, so we removed the bug label. But it makes sense to make it configurable, so we will provide this functionality, thx

@davismcphee davismcphee added the papercut Small "burr" in the product that we should fix. label Nov 14, 2024
@akowalska622 akowalska622 self-assigned this Nov 25, 2024
@akowalska622
Copy link
Contributor

akowalska622 commented Nov 26, 2024

Unassigning myself for now, as seems a bit more than a papercut (at least for a new hire :) ). I didn't want to burn too much time for this right now.
Below you can find my approach and encountered problems.

Approach

  1. Our primary focus should be kibana/src/plugins/discover/public/application/context/context_app_content.tsx
  2. Adding props (either one) onUpdateDataGridDensity, onUpdateRowHeight, onUpdateHeaderRowHeight, onUpdateSampleSize to DiscoverGridMemoized enables the button in the UI, which allows us to control the parameters if correct functions are passed down
    Image
  3. I wanted to handle it in the local state, so my initial approach was:
  const services = useDiscoverServices();
  const { uiSettings, uiActions } = services;

  const configRowHeight = uiSettings.get(ROW_HEIGHT_OPTION);

  const [rowHeight, setRowHeight] = useState<number>(configRowHeight);

Then passing rowHeight and setRowHeight to DiscoverGridMemoized

Problems

It correctly changes row height locally for Context view, however a couple of problems occurred and tracing them down seems a bit complicated as per my Kibana knowledge right now.

  1. After switching mode to auto fit and/or to single I can't switch back to custom
    Image

I noticed that this condition isn't firing (only on switching back to custom (even though both functions exist):
packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx

    settings.push(
      <RowHeightSettings
        rowHeight={rowHeight}
        rowHeightLines={rowHeightLines}
        label={i18n.translate('unifiedDataTable.rowHeightLabel', {
          defaultMessage: 'Cell row height',
        })}
        onChangeRowHeight={onChangeRowHeight}
        onChangeRowHeightLines={onChangeRowHeightLines}
        data-test-subj="unifiedDataTableRowHeightSettings"
      />
    );
  }
  1. If we change row height in Discover it influences row height in Context (correct behavior ✅). If we change row height in Context, it doesn't influence Discover view directly (correct ✅), but at the same time resets Discover view to default 3 rows (incorrect 🔴)

I'm happy to be back with this issue when I get a bit more experience in Kibana, unless the solution seems pretty obvious for someone more experienced - then I'm more than happy to receive some guidance here :)

@akowalska622 akowalska622 removed their assignment Nov 26, 2024
@davismcphee
Copy link
Contributor

Good investigation @akowalska622, and thanks for adding your notes! We can chat about it more offline if you'd like, but I also think it's ok to table this for now and revisit later on with a bit more familiarity with the codebase 🙂 It definitely seems like you were on the right track and pretty close to a solution, though. Besides the "Custom" height bug, it also seems like there may be a couple of sate related questions to work through as a team and agree on a good behaviour.

@akowalska622
Copy link
Contributor

Thanks @davismcphee! I'm more than happy to revisit it together 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Discover Discover Application impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort papercut Small "burr" in the product that we should fix. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
Projects
None yet
Development

No branches or pull requests

5 participants