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: ✨ seperate social media icons #1089

Closed
5 of 6 tasks
christophsaile opened this issue May 13, 2024 · 8 comments
Closed
5 of 6 tasks

feat: ✨ seperate social media icons #1089

christophsaile opened this issue May 13, 2024 · 8 comments
Assignees

Comments

@christophsaile
Copy link
Contributor

christophsaile commented May 13, 2024

User Story

The Authors in Celum want to seperate social media icons from the regular icons.

Suggested Solution

The following structure will be on the CDN
union-investment/

  • content/
  • content.json
  • system/
  • system.json
  • content-social/
  • content-socal.json
  • system-social/
  • system-social.json

In order that this works you would have to adjust your script with the following condition:
extend the condition with ... content-social || ... system-social

grafik

and add two new stories:

grafik

We could implement this feature.
Do you have any concerns impementing this @mariohamann, @yoezlem?

Subtasks

DoR

  • Item has business value
  • Item all subtasks have been estimated by the team
  • Item is clear and well-defined
  • Item dependencies have been identified

DoD

  • All subtasks have been closed
@yoezlem
Copy link
Contributor

yoezlem commented May 13, 2024

@mariohamann can you check if Chris missed something? CMS would work an that and would create a PR for this feat.

@mariohamann
Copy link
Contributor

We don't need a new feature here. You could easily use union-investment/content-social/instagram and it would just work out of the box. For content and social we created the "shortcut", as this is needed so often and is closely related to brand. content-social etc. feels to me more special and therefore I would skip the shortcut here – so that content-social/instagram doesn't work, but union-investment/content-social/instagram does – as it as already at the moment.

So for me this wouldn't be something that should be done, TBH.

@christophsaile
Copy link
Contributor Author

christophsaile commented May 14, 2024

@mariohamann I understand your point but since it does not add extra complexity it would be really nice to have this. Otherwise we would have to make a seperation on our side / magnolia side that we sometimes pass content/car.svg and sometimes else union-investment/content-social/facebook.svg which would make it more complex.
I think also for you it will make sense since with the upcoming celum release the social icons will not be part of the normal system.json / content.json anymore. Therefore if you don't change something / add new stories the social icons will not be displayed anymore in your preview

@mariohamann
Copy link
Contributor

mariohamann commented May 14, 2024

It's often easy to add features, but we have to maintain them and be careful with just adding things, if there are equal ways to do them.

In your case, you can just always add union-investment and your job is done — if you do it programmatically, this makes absolutely sense. :)

@christophsaile
Copy link
Contributor Author

@mariohamann this would mean that in the future (next week (?)) the SDS default icon library will not have social media icons for their users (since they will be delted from the system & content folder).
Unless of course you would nodify your script, or tell the users of the library that if they want to use social media icons they have to access them via union-investment/social-content/facebook.svg and all others via content/car.svg.
Tbh I don't think this is in the interest of the user and I disagree that this feature request will lead to more maintenance.

@mariohamann
Copy link
Contributor

In either case that would happen – even with the change you're proposing, users have to change the name of the icon from content/instagram to social-content/instagram.

I do definitely not see us fixing things when CELUM introduces breaking changes and it's explicitly not our responsibility.

That even strengthens my point, that I wouldn't introduce even more "shortcuts", that might break sometime or raise expectations on us.

So from my perspective:

  • CELUM introduces Breaking changes and there's nothing we can and should do about that from Solid perspective.
  • We introduced a shortcut for developers to avoid setting union-investment every time for the most important differentiation being content and system icons. Maybe that was a mistake, because apparently now new folders are popping up and there are expectations from us to add them.
  • You have everything in hand to overcome this by explicitly setting the complete path.

So the only thing you have to do is: In the occasions where social icons could put up, you could add the complete path – or you donit everywhere, where it can be set programmatically and you're totally safe.

@christophsaile
Copy link
Contributor Author

christophsaile commented May 15, 2024

@mariohamann Yes in both cases user have to change. But in our case they can access the icons in the same pattern as they are used to + we would provide a stories so its clear how to use them.

Since you already provided the shortcut, not providing the shortcut for the social icon makes the UX worse ... You woulnd't even have to do something, we would provide the change.
From Celum perspective it was clear from the beginning that the social media icons will be sepereated.

If you don't want to accept this feature I guess we are going to use the full path for all components, but be prepared that there will be users that are confuesd because they don't see any social icons in your stories + docs because social media icons are not only used by the CMS.

@christophsaile
Copy link
Contributor Author

FYI: we are now using the full path -> therefore I close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

No branches or pull requests

3 participants