chore: Add height/width TrashIcon SVG(18px/18px)-Edit Dataset modal #14956

andrewbastian · 2021-06-02T19:50:39Z

SUMMARY

Added fontSize: '18px'; to Trash Icons on Edit Dataset modal's 3 tabs which use this Icon. - metrics, columns, and calculated columns.

This was done inline to the SVG component via the CSS parameter. Also removed iconSize='m' parameter from Icon.Trash component.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE:

AFTER

UNCHANGED TRASH ICONS

TESTING INSTRUCTIONS

Attached above are screenshots of developer-tools showing the changes made. The layout box model shows that the height and width of the Trash SVG are set to 18px.

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 · 2021-06-02T20:44:11Z

Codecov Report

Merging #14956 (e7f9d62) into master (e82433e) will decrease coverage by 0.53%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #14956      +/-   ##
==========================================
- Coverage   77.78%   77.25%   -0.54%     
==========================================
  Files         966      969       +3     
  Lines       49629    49966     +337     
  Branches     6314     6432     +118     
==========================================
- Hits        38606    38600       -6     
- Misses      10822    11161     +339     
- Partials      201      205       +4

Flag	Coverage Δ
hive	`?`
javascript	`72.22% <100.00%> (-0.57%)`	⬇️
presto	`?`

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

Impacted Files	Coverage Δ
superset-frontend/src/CRUD/CollectionTable.tsx	`78.91% <100.00%> (-6.93%)`	⬇️
superset/db_engines/hive.py	`0.00% <0.00%> (-82.15%)`	⬇️
...trols/DateFilterControl/components/CommonFrame.tsx	`41.66% <0.00%> (-50.00%)`	⬇️
...et-frontend/src/dashboard/actions/nativeFilters.ts	`50.84% <0.00%> (-31.85%)`	⬇️
...nd/src/dashboard/components/nativeFilters/utils.ts	`56.25% <0.00%> (-30.00%)`	⬇️
...nents/nativeFilters/FilterBar/FilterSets/index.tsx	`47.00% <0.00%> (-30.00%)`	⬇️
...nd/src/dashboard/components/nativeFilters/state.ts	`75.00% <0.00%> (-25.00%)`	⬇️
...t-frontend/src/dashboard/reducers/nativeFilters.ts	`60.00% <0.00%> (-24.62%)`	⬇️
superset/db_engine_specs/hive.py	`70.32% <0.00%> (-17.08%)`	⬇️
...components/DashboardBuilder/DashboardContainer.tsx	`82.97% <0.00%> (-17.03%)`	⬇️
... and 78 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 e82433e...e7f9d62. Read the comment docs.

lyndsiWilliams · 2021-06-09T15:36:32Z

superset-frontend/src/CRUD/CollectionTable.tsx

@@ -261,7 +261,7 @@ export default class CRUDCollection extends React.PureComponent<
            aria-label="Delete item"
            className="pointer"
            data-test="crud-delete-icon"
-            iconSize="m"
+            css={{fontSize:"18px"}}


Can you try using style instead of css here? I think that's probably the best way to change this, check out this article.

The other thing you could try is to change iconSize to "s".

Thanks, @lyndsiWilliams and @AAfghahi! I Changed css to style. Based on the article you linked, Lyndsi, it looks like I was on the right track with using fontSize. I did try setting iconSize="l", but that only brought the icon up to 16px*16px and the ticket said 18px*18px.

Perfect, this looks great!

…rewbastian/trash_icon_style

…ndrewbastian/superset into andrewbastian/trash_icon_style

AAfghahi

This looks good! Thank you!

small nit: did you try it with iconSize="s"?

andrewbastian · 2021-06-09T17:58:40Z

This looks good! Thank you!

small nit: did you try it with iconSize="s"?

I did try that on the <Icon.Trash>, but it made the icon smaller. Am I missing something?

AAfghahi · 2021-06-09T18:00:12Z

This looks good! Thank you!
small nit: did you try it with iconSize="s"?

I did try that on the <Icon.Trash>, but it made the icon smaller. Am I missing something?

hmm, it might be fontSize 12. In which case this is probably the ideal solution.

lyndsiWilliams

Looks good to me! 😁

AAfghahi · 2021-06-15T02:42:37Z

@andrewbastian LGTM! @eschutho could you run the workflow for this one?

AAfghahi · 2021-06-15T18:48:28Z

superset-frontend/src/CRUD/CollectionTable.tsx

@@ -261,7 +261,7 @@ export default class CRUDCollection extends React.PureComponent<
            aria-label="Delete item"
            className="pointer"
            data-test="crud-delete-icon"
-            iconSize="m"
+            style={{fontSize: '18px'}}


Suggested change

style={{fontSize: '18px'}}

style={{fontSize: '18px' }}

can we change this to use the css prop instead of style? That's a pattern we've been trying to follow. Thanks!

eschutho · 2021-06-15T19:53:11Z

/testenv up

github-actions · 2021-06-15T21:45:57Z

@eschutho Ephemeral environment spinning up at http://35.166.235.161:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

eschutho · 2021-06-16T19:54:11Z

visual testing looks good 👍 Thanks @andrewbastian!

github-actions · 2021-06-18T15:50:32Z

Ephemeral environment shutdown and build artifacts deleted.

…pache#14956) * Add min-height/width TrashIcon SVG(18px/18px)-Edit Dataset modal * Reworked styling to be inline on SVG component via CSS from `emotion` * Changed parameter from `css` to 'style' * Fixed lint error. * Changed `style` back to `css` Co-authored-by: andrewbastian <andrewbastian@hosaka-deck.lan>

Add min-height/width TrashIcon SVG(18px/18px)-Edit Dataset modal

304e2a1

pull-request-size bot added the size/XS label Jun 2, 2021

hughhhh changed the title ~~Add min-height/width TrashIcon SVG(18px/18px)-Edit Dataset modal~~ chore: Add min-height/width TrashIcon SVG(18px/18px)-Edit Dataset modal Jun 2, 2021

andrewbastian marked this pull request as draft June 2, 2021 21:37

andrewbastian and others added 2 commits June 8, 2021 11:02

Reworked styling to be inline on SVG component via CSS from emotion

7b95d2e

Merge branch 'apache:master' into andrewbastian/trash_icon_style

85f823a

andrewbastian changed the title ~~chore: Add min-height/width TrashIcon SVG(18px/18px)-Edit Dataset modal~~ chore: Add height/width TrashIcon SVG(18px/18px)-Edit Dataset modal Jun 8, 2021

andrewbastian marked this pull request as ready for review June 8, 2021 18:27

lyndsiWilliams reviewed Jun 9, 2021

View reviewed changes

andrewbastian added 3 commits June 9, 2021 10:44

Changed parameter from css to 'style'

2656222

Merge branch 'master' of https://github.com//apache/superset into and…

6a876aa

…rewbastian/trash_icon_style

Merge branch 'andrewbastian/trash_icon_style' of https://github.com/a…

b15217f

…ndrewbastian/superset into andrewbastian/trash_icon_style

AAfghahi reviewed Jun 9, 2021

View reviewed changes

lyndsiWilliams approved these changes Jun 9, 2021

View reviewed changes

Fixed lint error.

4e4107f

AAfghahi approved these changes Jun 15, 2021

View reviewed changes

AAfghahi reviewed Jun 15, 2021

View reviewed changes

Changed style back to css

e7f9d62

eschutho approved these changes Jun 16, 2021

View reviewed changes

eschutho merged commit 2cf9946 into apache:master Jun 18, 2021

eschutho added the Superset-Community-Partners Preset community partner program participants label Jan 5, 2022

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Add height/width TrashIcon SVG(18px/18px)-Edit Dataset modal #14956

chore: Add height/width TrashIcon SVG(18px/18px)-Edit Dataset modal #14956

andrewbastian commented Jun 2, 2021 •

edited

Loading

codecov bot commented Jun 2, 2021 •

edited

Loading

lyndsiWilliams Jun 9, 2021

AAfghahi Jun 9, 2021 •

edited

Loading

andrewbastian Jun 9, 2021 •

edited

Loading

lyndsiWilliams Jun 9, 2021

AAfghahi left a comment

andrewbastian commented Jun 9, 2021

AAfghahi commented Jun 9, 2021

lyndsiWilliams left a comment

AAfghahi commented Jun 15, 2021

AAfghahi Jun 15, 2021

eschutho Jun 15, 2021

eschutho commented Jun 15, 2021

github-actions bot commented Jun 15, 2021

eschutho commented Jun 16, 2021

github-actions bot commented Jun 18, 2021

chore: Add height/width TrashIcon SVG(18px/18px)-Edit Dataset modal #14956

chore: Add height/width TrashIcon SVG(18px/18px)-Edit Dataset modal #14956

Conversation

andrewbastian commented Jun 2, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Jun 2, 2021 • edited Loading

Codecov Report

lyndsiWilliams Jun 9, 2021

Choose a reason for hiding this comment

AAfghahi Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

andrewbastian Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

lyndsiWilliams Jun 9, 2021

Choose a reason for hiding this comment

AAfghahi left a comment

Choose a reason for hiding this comment

andrewbastian commented Jun 9, 2021

AAfghahi commented Jun 9, 2021

lyndsiWilliams left a comment

Choose a reason for hiding this comment

AAfghahi commented Jun 15, 2021

AAfghahi Jun 15, 2021

Choose a reason for hiding this comment

eschutho Jun 15, 2021

Choose a reason for hiding this comment

eschutho commented Jun 15, 2021

github-actions bot commented Jun 15, 2021

eschutho commented Jun 16, 2021

github-actions bot commented Jun 18, 2021

andrewbastian commented Jun 2, 2021 •

edited

Loading

codecov bot commented Jun 2, 2021 •

edited

Loading

AAfghahi Jun 9, 2021 •

edited

Loading

andrewbastian Jun 9, 2021 •

edited

Loading