Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix icon titles #1809

Merged
merged 9 commits into from
Nov 26, 2019
Merged

Fix icon titles #1809

merged 9 commits into from
Nov 26, 2019

Conversation

bjoernricks
Copy link
Contributor

@bjoernricks bjoernricks commented Nov 26, 2019

Finally fix the icon titles.

Checklist:

Allow to use the render props pattern with the SvgIcon component. With
this change it will be possible to apply the title to a children
element easily.
In production setups the created svg icon in the DOM always gets a
<title> element. With this change the content of this element is set to
the same text as the outside <span> of <SvgIcon>.

This issue got introduced with
facebook/create-react-app#7118 and
facebook/create-react-app#7103
The snapshot tests of the icons are mostly useless. Therefore extend the
icon tests instead of relying on the snapshots.
With the latest changes the svg icon components will get a title
property. In tests this results in adding a title attribute and in
production it will add a title sub element.
We should always use past terms.
Removed old and not correct entry too.
@bjoernricks bjoernricks marked this pull request as ready for review November 26, 2019 14:05
@bjoernricks bjoernricks requested a review from sarahd93 November 26, 2019 14:06
@codecov
Copy link

codecov bot commented Nov 26, 2019

Codecov Report

Merging #1809 into gsa-9.0 will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           gsa-9.0    #1809      +/-   ##
===========================================
+ Coverage    47.54%   47.58%   +0.04%     
===========================================
  Files         1049     1049              
  Lines        24251    24273      +22     
  Branches      6837     6838       +1     
===========================================
+ Hits         11529    11551      +22     
  Misses       11572    11572              
  Partials      1150     1150
Impacted Files Coverage Δ
gsa/src/web/components/icon/withSvgIcon.js 100% <100%> (ø) ⬆️
gsa/src/web/components/icon/testing.js 100% <100%> (ø) ⬆️
gsa/src/web/components/icon/svgicon.js 91.42% <100%> (+0.51%) ⬆️

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 fe66448...f3c06bc. Read the comment docs.

gsa/src/web/components/icon/testing.js Outdated Show resolved Hide resolved
Co-Authored-By: sarahd93 <sarah.diedrich93@gmail.com>
@bjoernricks bjoernricks requested a review from sarahd93 November 26, 2019 14:50
@bjoernricks bjoernricks merged commit 0778908 into greenbone:gsa-9.0 Nov 26, 2019
@bjoernricks bjoernricks deleted the fix-icon-titles branch November 26, 2019 15:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants