-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix: Partially enhance event dashboard #5139
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/dffzbk86t |
app/styles/pages/public-event.scss
Outdated
@@ -185,3 +185,7 @@ | |||
.event-map > iframe.g-map { | |||
height: 300px; | |||
} | |||
|
|||
.content a { | |||
text-transform: capitalize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will capitalize all links in any content container on any page, and not just social media links on event detail page as listed in the issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iamareebjamal i will fixed this
Codecov Report
@@ Coverage Diff @@
## development #5139 +/- ##
===============================================
+ Coverage 22.65% 23.24% +0.59%
===============================================
Files 489 493 +4
Lines 5191 5170 -21
Branches 36 38 +2
===============================================
+ Hits 1176 1202 +26
+ Misses 4010 3963 -47
Partials 5 5
Continue to review full report at Codecov.
|
6c9340c
to
4653995
Compare
4935c79
to
5abdd53
Compare
@iamareebjamal @mariobehling plz review i fixed this bug with id |
This pull request introduces 1 alert when merging 9b98ab7 into e044006 - view on LGTM.com new alerts:
|
Here is an overview of what got changed by this pull request: Issues
======
- Added 2
See the complete overview on Codacy |
app/models/social-link.js
Outdated
|
||
export default ModelBase.extend({ | ||
name : attr('string'), | ||
link : attr('string'), | ||
identifier : attr('string'), // used for providing css id for URL validations. | ||
|
||
event: belongsTo('event'), | ||
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codacy found an issue: Trailing spaces not allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't resolve the conversations. They are resolved automatically
app/models/social-link.js
Outdated
@@ -4,14 +4,15 @@ import attr from 'ember-data/attr'; | |||
import ModelBase from 'open-event-frontend/models/base'; | |||
import { belongsTo } from 'ember-data/relationships'; | |||
import { computedSegmentedLink, socialPlatforms } from 'open-event-frontend/utils/computed-helpers'; | |||
import { capitalize } from 'lodash-es'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codacy found an issue: 'capitalize' is defined but never used.
This pull request introduces 1 alert when merging 1470cd9 into e044006 - view on LGTM.com new alerts:
|
app/models/social-link.js
Outdated
@@ -31,4 +31,4 @@ export default ModelBase.extend({ | |||
}), | |||
|
|||
segmentedLink: computedSegmentedLink.bind(this)('link') | |||
}); | |||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}); | |
}); | |
app/models/social-link.js
Outdated
@@ -11,7 +11,6 @@ export default ModelBase.extend({ | |||
identifier : attr('string'), // used for providing css id for URL validations. | |||
|
|||
event: belongsTo('event'), | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you delete the line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iamareebjamal according to this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a difference between a space and a line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iamareebjamal now no error
Co-authored-by: Mario Behling <mb@mariobehling.de>
Short description of what this resolves:
in this pr i fixed the points given below--
Fixes #5131
screenshot before-
screenshot after-
Checklist
development
branch.@iamareebjamal plz review now I fixed 3 points