fix: Show online or mixed location in event page header #5196

HADES-01 · 2020-09-30T11:10:15Z

Fixes #5161

Short description of what this resolves:

Fixed the issue of empty location line in the header of the public event page.

Changes proposed in this pull request:

Fixed by creating a computed property as suggested by @iamareebjamal

Checklist

I have read the Contribution & Best practices Guide.
My branch is up-to-date with the Upstream development branch.
The acceptance, integration, unit tests and linter pass locally with my changes

vercel · 2020-09-30T11:10:25Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/hvcla1qc7
✅ Preview: https://open-event-frontend-git-fork-hades-01-issue5161.eventyay.vercel.app

iamareebjamal · 2020-09-30T11:24:16Z

app/models/event.js

+    if (this.locationName && this.online) {
+      return `In-Person Event and Online Event ${this.locationName}`;
+    }
+    else if (!this.locationName && this.online) {


Suggested change

else if (!this.locationName && this.online) {

else if (this.online) {

iamareebjamal · 2020-09-30T11:24:27Z

app/models/event.js

+    else if (!this.locationName && this.online) {
+      return 'Online Event';
+    }
+    else if (!this.online && this.locationName) {


Suggested change

else if (!this.online && this.locationName) {

else if (this.locationName) {

codecov · 2020-09-30T11:24:42Z

Codecov Report

Merging #5196 into development will decrease coverage by 0.03%.
The diff coverage is 0.00%.

@@               Coverage Diff               @@
##           development    #5196      +/-   ##
===============================================
- Coverage        22.65%   22.62%   -0.04%     
===============================================
  Files              489      489              
  Lines             5191     5198       +7     
  Branches            36       36              
===============================================
  Hits              1176     1176              
- Misses            4010     4017       +7     
  Partials             5        5

Impacted Files	Coverage Δ
app/controllers/public.js	`0.00% <0.00%> (ø)`

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 e90d869...17e0c5e. Read the comment docs.

iamareebjamal · 2020-09-30T11:24:42Z

app/models/event.js

@@ -181,6 +181,21 @@ export default class Event extends ModelBase.extend(CustomPrimaryKeyMixin, {

  sessionsByState: computed('sessions', function() {
    return groupBy(this.sessions.toArray(), 'data.state');
+  }),
+
+  headererLocation: computed('locationName', 'online', function() {


Suggested change

headererLocation: computed('locationName', 'online', function() {

headerLocation: computed('locationName', 'online', function() {

iamareebjamal · 2020-09-30T11:25:10Z

Use correct formatting

app/models/event.js

iamareebjamal · 2020-09-30T11:28:20Z

app/models/event.js

@@ -181,6 +181,18 @@ export default class Event extends ModelBase.extend(CustomPrimaryKeyMixin, {

  sessionsByState: computed('sessions', function() {
    return groupBy(this.sessions.toArray(), 'data.state');
+  }),
+
+  headerLocation: computed('locationName', 'online', function() {


Move this to the controller rather than the model

iamareebjamal · 2020-09-30T12:01:55Z

app/templates/public.hbs

@@ -14,7 +14,7 @@
              <h5 class="event time ends">{{t 'To'}} {{header-date this.model.endsAt this.model.timezone}}</h5>
            {{/if}}
            <h1 class="event name">{{this.model.name}}</h1>
-            <h4 class="event location"><i class="icon map marker alternate"></i>{{this.model.locationName}}</h4>
+            <h4 class="event location"><i class="icon map marker alternate"></i>{{this.model.headerLocation}}</h4>


Suggested change

<h4 class="event location"><i class="icon map marker alternate"></i>{{this.model.headerLocation}}</h4>

<h4 class="event location"><i class="icon map marker alternate"></i>{{this.headerLocation}}</h4>

Do not resolve conversation. It is resolved automatically once suggested changes are made

iamareebjamal

Wrong reference

iamareebjamal · 2020-09-30T13:42:31Z

app/controllers/public.js

@@ -20,6 +20,19 @@ export default class PublicController extends Controller {
    return this.session.currentRouteName && this.session.currentRouteName !== 'public.cfs.new-session' && this.session.currentRouteName !== 'public.cfs.new-speaker' && this.session.currentRouteName !== 'public.cfs.edit-speaker' && this.session.currentRouteName !== 'public.cfs.edit-session';
  }

+  @computed('locationName', 'online')


Wrong computed properties

Every event is shown as Online Event. Please test the PR properly

@iamareebjamal It shows online event at first, but when you refresh it shows the correct value. Then this value shows up for the next event you click. Can you help me, I am not able to figure it out myself.

@computed('model.locationName', 'model.online')

@iamareebjamal Thanks

vercel bot deployed to Preview September 30, 2020 11:10 View deployment

iamareebjamal reviewed Sep 30, 2020

View reviewed changes

HADES-01 force-pushed the issue#5161 branch from 36f5308 to 177dfe9 Compare September 30, 2020 11:26

vercel bot deployed to Preview September 30, 2020 11:26 View deployment

iamareebjamal reviewed Sep 30, 2020

View reviewed changes

app/models/event.js Outdated Show resolved Hide resolved

iamareebjamal reviewed Sep 30, 2020

View reviewed changes

app/models/event.js Outdated Show resolved Hide resolved

iamareebjamal reviewed Sep 30, 2020

View reviewed changes

vercel bot temporarily deployed to Preview September 30, 2020 11:32 Inactive

vercel bot deployed to Preview September 30, 2020 11:32 View deployment

HADES-01 force-pushed the issue#5161 branch from f35a200 to 7dfbe89 Compare September 30, 2020 11:43

vercel bot deployed to Preview September 30, 2020 11:43 View deployment

iamareebjamal changed the title ~~Fixed Public Event Page Location Issue #5161~~ fix: Show online or mixed location in event page header Sep 30, 2020

iamareebjamal approved these changes Sep 30, 2020

View reviewed changes

auto-label bot added the fix label Sep 30, 2020

iamareebjamal reviewed Sep 30, 2020

View reviewed changes

iamareebjamal requested changes Sep 30, 2020

View reviewed changes

vercel bot temporarily deployed to Preview September 30, 2020 12:04 Inactive

vercel bot deployed to Preview September 30, 2020 12:05 View deployment

Fixed Public Event Page Location Issue fossasia#5161

5ff36be

HADES-01 force-pushed the issue#5161 branch from e8867be to 5ff36be Compare September 30, 2020 12:20

vercel bot deployed to Preview September 30, 2020 12:20 View deployment

Fixed Public Event Page Location Issue fossasia#5161

d65fe06

vercel bot deployed to Preview September 30, 2020 12:26 View deployment

Update public.hbs

946f499

vercel bot deployed to Preview September 30, 2020 12:33 View deployment

HADES-01 requested a review from iamareebjamal September 30, 2020 12:47

iamareebjamal reviewed Sep 30, 2020

View reviewed changes

Update public.js

17e0c5e

vercel bot deployed to Preview September 30, 2020 14:04 View deployment

HADES-01 requested a review from iamareebjamal September 30, 2020 14:20

iamareebjamal merged commit e0e232f into fossasia:development Sep 30, 2020

HADES-01 deleted the issue#5161 branch September 30, 2020 18:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Show online or mixed location in event page header #5196

fix: Show online or mixed location in event page header #5196

HADES-01 commented Sep 30, 2020 •

edited

vercel bot commented Sep 30, 2020 •

edited

iamareebjamal Sep 30, 2020

iamareebjamal Sep 30, 2020

codecov bot commented Sep 30, 2020 •

edited

iamareebjamal Sep 30, 2020

iamareebjamal commented Sep 30, 2020

iamareebjamal Sep 30, 2020

iamareebjamal Sep 30, 2020

iamareebjamal Sep 30, 2020

iamareebjamal left a comment

iamareebjamal Sep 30, 2020

HADES-01 Sep 30, 2020

iamareebjamal Sep 30, 2020

HADES-01 Sep 30, 2020

	else if (!this.locationName && this.online) {
	else if (this.online) {

	else if (!this.online && this.locationName) {
	else if (this.locationName) {

	headererLocation: computed('locationName', 'online', function() {
	headerLocation: computed('locationName', 'online', function() {

	<h4 class="event location"><i class="icon map marker alternate"></i>{{this.model.headerLocation}}</h4>
	<h4 class="event location"><i class="icon map marker alternate"></i>{{this.headerLocation}}</h4>

fix: Show online or mixed location in event page header #5196

fix: Show online or mixed location in event page header #5196

Conversation

HADES-01 commented Sep 30, 2020 • edited

Short description of what this resolves:

Changes proposed in this pull request:

Checklist

vercel bot commented Sep 30, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 30, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

iamareebjamal commented Sep 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamareebjamal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HADES-01 commented Sep 30, 2020 •

edited

vercel bot commented Sep 30, 2020 •

edited

codecov bot commented Sep 30, 2020 •

edited