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

chore: customize adhoc filter icon and fix creatable label #14896

Merged
merged 3 commits into from
May 31, 2021

Conversation

villebro
Copy link
Member

SUMMARY

Harmonize AntD Select components that are used in Explore/Dashboard view. This changes the following:

  • Changes AdHoc Filter Select to show the StopOutline Icon if "NOT IN" operator is selected. This is to align the behavior with the native select filter polish introduced in feat(native-filters): improve inverse selection indicators #14873
  • Show "Create: " instead of "" when given the option to create a select option that doesn't exist in the dataset. This is to mimic the behavior of the react-select based component used in Filter Box: image

BEFORE

When "NOT IN" is selected in the adhoc filter popover, we currently use the default CheckOutlined icon:
image
When being presented with the option to create a value that doesn't exist in the dataset, the option to use a value that doesn't exist is shown as-is:
image

AFTER

When "NOT IN" is selected in the adhoc filter popover, we now use the StopOutlined
image
When being presented with the option to create a value that doesn't exist in the dataset, The creatable value is prefixed with "Create" and the value placed inside quotes:
image

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@codecov
Copy link

codecov bot commented May 28, 2021

Codecov Report

Merging #14896 (995ef74) into master (d86880d) will decrease coverage by 0.11%.
The diff coverage is 61.90%.

❗ Current head 995ef74 differs from pull request most recent head 8a5eff4. Consider uploading reports for the commit 8a5eff4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14896      +/-   ##
==========================================
- Coverage   77.51%   77.39%   -0.12%     
==========================================
  Files         963      963              
  Lines       49315    49316       +1     
  Branches     6226     6228       +2     
==========================================
- Hits        38225    38167      -58     
- Misses      10889    10950      +61     
+ Partials      201      199       -2     
Flag Coverage Δ
hive ?
javascript 72.46% <61.90%> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
superset-frontend/src/constants.ts 100.00% <ø> (ø)
superset-frontend/src/dashboard/actions/hydrate.js 1.72% <0.00%> (-0.02%) ⬇️
.../components/Header/HeaderActionsDropdown/index.jsx 68.42% <ø> (ø)
...rontend/src/explore/components/EmbedCodeButton.jsx 80.76% <ø> (ø)
...nd/src/explore/components/ExploreViewContainer.jsx 2.15% <0.00%> (ø)
superset-frontend/src/modules/utils.js 83.33% <ø> (+30.70%) ⬆️
superset-frontend/src/utils/urlUtils.ts 53.33% <33.33%> (-8.21%) ⬇️
superset-frontend/src/components/Menu/Menu.tsx 66.66% <100.00%> (+0.40%) ⬆️
...d/components/DashboardBuilder/DashboardBuilder.tsx 87.09% <100.00%> (+0.14%) ⬆️
...set-frontend/src/dashboard/util/getDashboardUrl.ts 91.66% <100.00%> (ø)
... and 12 more

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 d86880d...8a5eff4. Read the comment docs.

@@ -417,6 +418,9 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon
onSearch={val => this.setState({ currentSuggestionSearch: val })}
onSelect={this.clearSuggestionSearch}
onBlur={this.clearSuggestionSearch}
menuItemSelectedIcon={
operator === 'NOT IN' ? <StopOutlined /> : <CheckOutlined />
Copy link
Member

Choose a reason for hiding this comment

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

<Icons.StopOutlined> and <Icons.CheckOutlined>

Copy link
Member

@geido geido left a comment

Choose a reason for hiding this comment

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

LGTM!

<Icons.StopOutlined iconSize="m" />
) : (
<Icons.CheckOutlined iconSize="m" />
)
Copy link
Contributor

Choose a reason for hiding this comment

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

May be shorter will be and also without duplication :)

const Icon = inverseSelection ? Icons.StopOutlined : Icons.CheckOutlined
...
menuItemSelectedIcon={<Icon iconSize="m" />}

@villebro villebro merged commit 66282c3 into apache:master May 31, 2021
@villebro villebro deleted the villebro/select-polish branch May 31, 2021 06:45
villebro added a commit to preset-io/superset that referenced this pull request Jun 2, 2021
)

* chore: customize adhoc filter icon and fix creatable label

* use common icon component

* simplify code
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
)

* chore: customize adhoc filter icon and fix creatable label

* use common icon component

* simplify code
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
)

* chore: customize adhoc filter icon and fix creatable label

* use common icon component

* simplify code
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
)

* chore: customize adhoc filter icon and fix creatable label

* use common icon component

* simplify code
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io size/S 🚢 1.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants