chore(explore): Hide non-droppable metric and column list #27717
Conversation
justinpark
requested review from
rusackas,
john-bodley,
eschutho and
michael-s-molina
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #27717 +/- ##
==========================================
+ Coverage 67.47% 67.50% +0.02%
==========================================
Files 1911 1913 +2
Lines 75020 75142 +122
Branches 8355 8403 +48
==========================================
+ Hits 50621 50722 +101
- Misses 22349 22358 +9
- Partials 2050 2062 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| const allowedColumns = useMemo(() => { | ||
| const validators = Object.values(dropzones); | ||
| if (!isArray(_columns)) return []; | ||
| return allowlistOnly |
There was a problem hiding this comment.
@justinpark If we have the concept of validators where every item is checked to see if it's allowed, similar to a security check, wouldn't be the responsibility of the validator to check for disallow_adhoc_metrics? In other words, could we always validate the items with the default implementation being that all items are allowed? This would give us the flexibility to show/hide items depending on any type of condition.
There was a problem hiding this comment.
In other words, could we always validate the items with the default implementation being that all items are allowed?
@michael-s-molina As mentioned earlier, in the adhoc query, all columns and metrics are accepted. Therefore, applying additional filtering in cases where non-disallowed-adhoc case can be considered unnecessary.
There was a problem hiding this comment.
We synced offline and will ensure that these filters are not affected by the 'disallow-adhoc' condition.
john-bodley
added
the
review:checkpoint
justinpark
changed the title
justinpark
force-pushed
the
chore--explore-filter-out-ineligible-list
branch
from
92ea7fe to
7fb999a
Compare
john-bodley
removed
the
review:checkpoint
|
/testenv up |
|
@justinpark I think it would be important to show some kind of message/feedback to users when we have hidden columns or metrics because they can't be used anywhere. Maybe @kasiazjc can help us here. |
|
@michael-s-molina If you are interested in considering feedback, this proposal suggests adopting a Gmail-style approach. cc: @kasiazjc |
|
This looks nice @justinpark. I would only keep the background white to not mix it with the draggable items and because this looks like a secondary information that does not need to be highlighted frequently. Once users view this for the first time, they will get used to it. We won't have the Undo link right? I'm assuming that you can't undo the applicability of columns because they depend on what's already configured in the controls. |
Sounds good. I'll update accordingly.
I agree with your opinion. Let me remove "undo" now. |
|
/testenv up |
|
@michael-s-molina Ephemeral environment spinning up at http://35.94.26.119:8080. Credentials are |
|
@justinpark If you resize the left panel, it's cutting the text. 
|
@michael-s-molina I fixed by the text ellipsis.
|
|
Ephemeral environment shutdown and build artifacts deleted. |
john-bodley
added
the
v4.0
(cherry picked from commit eda304b)
mistercrunch
added
🍒 4.0.1
🍒 4.0.2
🏷️ bot
(cherry picked from commit c06f068)
SUMMARY
There can be certain items that cannot be accepted in all options. (For instance, metrics cannot be accepted in the columns selection. In cases where the controls solely consist of the columns selection, none of the metrics will be accepted in the overall dropzone, as shown in the screenshot below.)
To address this, the commit takes a step forward by checking the eligibility of each filter and hiding the item when there are no eligible options available.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
before--hide-disallow-list.mov
After:
after--hide-disallow-list.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION