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

dimension list: hide sub-dimensions for fields that are FK #17783

Merged
merged 1 commit into from
Sep 8, 2021

Conversation

pawit-metabase
Copy link
Contributor

This is likely a regression from #14897 treating FK as sub-dimensions of the FK field (6bdd5e0#diff-7af510db40767873db52e7ebfd7cdeb5b4918d10ef08dfa2e998dab79fe66741R725) which DimensionList then later pick up to display.

Fixes #16787

@codecov
Copy link

codecov bot commented Sep 7, 2021

Codecov Report

Merging #17783 (e351b43) into master (9a9a987) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #17783   +/-   ##
=======================================
  Coverage   62.90%   62.90%           
=======================================
  Files        1630     1633    +3     
  Lines       66581    66608   +27     
  Branches     7304     7311    +7     
=======================================
+ Hits        41882    41900   +18     
- Misses      21510    21517    +7     
- Partials     3189     3191    +2     
Flag Coverage Δ
front-end 38.89% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
...etabase/query_builder/components/DimensionList.jsx 56.16% <100.00%> (-1.37%) ⬇️
...components/CollapseSection/CollapseSection.info.js 0.00% <0.00%> (ø)
...ase/components/CollapseSection/CollapseSection.jsx 75.00% <0.00%> (ø)
...mponents/CollapseSection/CollapseSection.styled.js 100.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 9a9a987...e351b43. Read the comment docs.

Comment on lines +79 to +80
.find(".Field-extra")
.should("not.have.descendants", "*");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I like this very much! Elegant solution 👌
FWIW, we shouldn't even display the .Field-extra as an empty div, but that's out of the scope of this PR.

Great work, @pawit-metabase !

Copy link
Member

@kulyk kulyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pawit-metabase pawit-metabase merged commit 2ffb8cb into master Sep 8, 2021
@pawit-metabase pawit-metabase deleted the hide-sub-dimension-fk branch September 8, 2021 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FK fields shows sub-dimension selector
4 participants