fix: adding extra social link #5144
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/a79g4syv9 |
|
Well, when adding new social links it would be good to have them here the way it is needed. We dont need a function to make all small case or caps. We just need the social links to be written in the way that the brands do it, e.g. YouTube, VK. I am aware that this is another issue for existing links, but you are introducing new social links that do not appear as expected. |
| this.set('segmentedLink', { | ||
| protocol : `https://${link}.com/u/`, | ||
| address : '' | ||
| }); |
There was a problem hiding this comment.
This won't do. It is already highly complex, this should go as a prefix
There was a problem hiding this comment.
should just left as https://${link}.com/ for all
There was a problem hiding this comment.
No, this should not happen here at all
Codecov Report
@@ Coverage Diff @@
## development #5144 +/- ##
===============================================
- Coverage 22.79% 22.71% -0.08%
===============================================
Files 489 489
Lines 5173 5181 +8
Branches 35 35
===============================================
- Hits 1179 1177 -2
- Misses 3989 3999 +10
Partials 5 5
Continue to review full report at Codecov.
|
|
@mariobehling Those are internal identifiers to be saved in DB, thus need to be kept in consistent lower casing for matching purposes, there should be another mapping of the identifiers to proper cased variants |
So, that should be done in a separate issue? Please proceed as you see fit. |
|
No it will be done in the same PR |
|
Oh sorry, I thought this PR is about changing Yes, the mapping changes will be done in that PR, not this one |
| @@ -31,7 +31,7 @@ export default class LinkInput extends Component { | |||
|
|
|||
| @observes('protocol', 'address') | |||
| protocolAddressObserver() { | |||
| const link = this.linkName?.toLowerCase(); | |||
| const link = this.linkName; | |||
There was a problem hiding this comment.
Restore the change please
iamareebjamal
changed the title
Fixes #5142
Checklist
developmentbranch.