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

Feat: Add a custom icon #17241

Merged
merged 7 commits into from
Jul 13, 2023
Merged

Feat: Add a custom icon #17241

merged 7 commits into from
Jul 13, 2023

Conversation

Dedelweiss
Copy link
Contributor

@Dedelweiss Dedelweiss commented Apr 13, 2023

In this PR, I'm adding the new custom icons feature. This allows users to add one or more custom icons (in png or svg format).

  • Add custom
  • Src from the CSS file
  • Fix size for png
  • Multiple custom icons
  • The user must use a name in lowercase for custom icon
  • Add tests

Code:

image

IE:
Dark icon is optional so you can just put:
"-social-links:custom::https://joyofcoding.org/sponsoring.html::joycoding.png,custom::lucasleblanc.com::lucasLogo.svg,github::https://github.com/Dedelweiss"

Result:

Screenshot 2023-06-29 at 16 16 48

I think a next Issue/PR will be needed to improve the tap target of the footer's icons.

Fixes: #16856

@Dedelweiss Dedelweiss marked this pull request as ready for review April 18, 2023 15:11
@Dedelweiss
Copy link
Contributor Author

Thank you very much @KacperFKorban for the feedback. Just added the corrections that you sent to me, now I'm looking for a better dynamic src for the images.

@ckipp01
Copy link
Member

ckipp01 commented May 11, 2023

One more thing to think about here after speaking with you @Dedelweiss about this today. Keep in mind that with the solution that you outlined it will only work for 1 custom icon. So if the user wants two, then it falls apart. I think whatever solution we come up with it should allow for X amount of custom icons/links. Like the number shouldn't matter.

@Dedelweiss
Copy link
Contributor Author

After checking, the functionality already allows the addition of several custom links/logos. But I haven't found a way to associate this with a url in the css file path. Do you have any examples that I can use to guide me @Florian3k ? Thank you very much

@Florian3k
Copy link
Contributor

I'm not a CSS expert, but maybe it's possible to do something like this:
In CSS file add this fragment:

.icon-button.custom::after {
  content: attr(data-custom-icon url);
}

And in the renderer add this data-custom-icon attribute to the button:

<button data-custom-icon="../../../../path/to/image.svg">

This could possibly be extended for more attributes for dark icon etc.

@Florian3k
Copy link
Contributor

I just realised that support for this is practically nonexistent: https://caniuse.com/mdn-css_types_attr_type-or-unit_url
I'll try to figure out something else. I may be necessary to move those url's from css to src attributes for images in html.

@Dedelweiss
Copy link
Contributor Author

I just realised that support for this is practically nonexistent: https://caniuse.com/mdn-css_types_attr_type-or-unit_url I'll try to figure out something else. I may be necessary to move those url's from css to src attributes for images in html.

Thank you @Florian3k, I will look at it too, see if it's possible to do something different.

Dedelweiss and others added 3 commits June 28, 2023 14:30
- Add setting "custom"
- Possible white icon with||without dark icon
- Improve tap target
- Change SocialLinks and do a match case
- Rephrase ScaladocSetting for custom
@Dedelweiss Dedelweiss force-pushed the cutom_link branch 3 times, most recently from d694e04 to 27b0f60 Compare June 29, 2023 14:37
Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

Just a few nits

@Dedelweiss Dedelweiss requested a review from ckipp01 July 10, 2023 14:42
Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

Just one nit on adding a test, but other than that this should be good to go.

scaladoc/src/dotty/tools/scaladoc/SocialLinks.scala Outdated Show resolved Hide resolved
@Dedelweiss Dedelweiss requested a review from ckipp01 July 13, 2023 07:53
Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thanks @Dedelweiss!

Copy link
Member

@KacperFKorban KacperFKorban left a comment

Choose a reason for hiding this comment

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

Looks good now, one last nitpick from me.

scaladoc/src/dotty/tools/scaladoc/SocialLinks.scala Outdated Show resolved Hide resolved
@ckipp01 ckipp01 enabled auto-merge (rebase) July 13, 2023 14:13
@ckipp01 ckipp01 merged commit a9f4008 into scala:main Jul 13, 2023
@andrew-selvia
Copy link

Thank you @Dedelweiss!! 🙌🏻

@Kordyjan Kordyjan added this to the 3.4.0 milestone Aug 2, 2023
Kordyjan added a commit that referenced this pull request Dec 8, 2023
Backports #17241 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
@Kordyjan Kordyjan modified the milestones: 3.4.0, 3.3.2 Dec 14, 2023
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.

scaladoc3 social links cannot be custom
6 participants