Skip to content

Conversation

@bendichter
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Mar 19, 2025

Deploy Preview for nwborg ready!

Name Link
🔨 Latest commit bc5f40f
🔍 Latest deploy log https://app.netlify.com/sites/nwborg/deploys/67de4f2f4cab2300083a3843
😎 Deploy Preview https://deploy-preview-23--nwborg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rly
Copy link
Contributor

rly commented Mar 20, 2025

It would be nice if the title of the event was clickable (in addition to being able to click the View Details button)

image

@rly
Copy link
Contributor

rly commented Mar 20, 2025

Could you add date, time, and location in a box on the right of each event page?

image

The date and location under the image do not include the time and the room, which you have to scroll down to find. I think it would be nice to have that easily findable especially since there is space on the right.

@rly
Copy link
Contributor

rly commented Mar 20, 2025

Overall, this looks great!

@bendichter bendichter requested a review from rly March 20, 2025 01:01
@bendichter
Copy link
Contributor Author

@rly I addressed your comments

Comment on lines +23 to +57
<div class="d-flex flex-wrap gap-4 mb-4">
{{ with .Date }}
<div class="d-flex gap-2 align-items-center">
<div class="icons">
<i class="fas fa-calendar-alt text-primary" style="font-size: 1.2rem;"></i>
</div>
<p class="inter-400 text-16 dark-600 mb-0 pt-1">
{{ . | time.Format "January 02, 2006" }}
{{ if $.Params.endDate }}
{{ $startDate := . | time.Format "2006-01-02" }}
{{ $endDate := time.AsTime $.Params.endDate | time.Format "2006-01-02" }}
{{ if ne $startDate $endDate }}
- {{ time.AsTime $.Params.endDate | time.Format "January 2, 2006" }}
{{ end }}
{{ end }}
</p>
</div>
{{ end }}
{{ with .Params.location }}
<div class="d-flex gap-2 align-items-center">
<div class="icons">
<i class="fas fa-map-marker-alt text-primary" style="font-size: 1.2rem;"></i>
</div>
<p class="inter-400 text-16 dark-600 mb-0 pt-1">{{ . }}</p>
</div>
{{ end }}
{{ with .Params.eventType }}
<div class="d-flex gap-2 align-items-center">
<div class="icons">
<i class="fas fa-tag text-primary" style="font-size: 1.2rem;"></i>
</div>
<p class="inter-400 text-16 dark-600 mb-0 pt-1">{{ . }}</p>
</div>
{{ end }}
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the info on the right! Thanks. Now I feel like this is kind of redundant and could be removed. Up to you though.

Suggested change
<div class="d-flex flex-wrap gap-4 mb-4">
{{ with .Date }}
<div class="d-flex gap-2 align-items-center">
<div class="icons">
<i class="fas fa-calendar-alt text-primary" style="font-size: 1.2rem;"></i>
</div>
<p class="inter-400 text-16 dark-600 mb-0 pt-1">
{{ . | time.Format "January 02, 2006" }}
{{ if $.Params.endDate }}
{{ $startDate := . | time.Format "2006-01-02" }}
{{ $endDate := time.AsTime $.Params.endDate | time.Format "2006-01-02" }}
{{ if ne $startDate $endDate }}
- {{ time.AsTime $.Params.endDate | time.Format "January 2, 2006" }}
{{ end }}
{{ end }}
</p>
</div>
{{ end }}
{{ with .Params.location }}
<div class="d-flex gap-2 align-items-center">
<div class="icons">
<i class="fas fa-map-marker-alt text-primary" style="font-size: 1.2rem;"></i>
</div>
<p class="inter-400 text-16 dark-600 mb-0 pt-1">{{ . }}</p>
</div>
{{ end }}
{{ with .Params.eventType }}
<div class="d-flex gap-2 align-items-center">
<div class="icons">
<i class="fas fa-tag text-primary" style="font-size: 1.2rem;"></i>
</div>
<p class="inter-400 text-16 dark-600 mb-0 pt-1">{{ . }}</p>
</div>
{{ end }}
</div>

@rly
Copy link
Contributor

rly commented Mar 20, 2025

Thanks! I made one suggestions above - feel free to take it or leave it

@oruebel
Copy link
Contributor

oruebel commented Mar 20, 2025

The event listing looks nice. However, I wasn't able to view the actual event pages. In the preview, when selecting any of the events 'View Details' I get a "404 Error: page not found!" .

For events that used the nwb_hackathons repo, should we add a link to the original GitHub source since some events have additional documents in the GitHub repo? This is not critical and shouldn't hold up the PR, I'm just wondering whether that would be useful to make it easy to find the original pages for old events.

@rly
Copy link
Contributor

rly commented Mar 21, 2025

@oruebel I fixed the event detail pages in the deployment preview (the base URL was wrong in the preview but you could have changed the base URL manually)

@oruebel
Copy link
Contributor

oruebel commented Mar 21, 2025

Just for completeness, it would be nice to also add an entry for:

  • 2nd Hackathon: May 2015 - May 14-16, 2015, Janelia Farm, in Ashburn, Virginia.
    
  • [1st Hackathon:](http://crcns.org/NWB/hackathon-1): November 20-22, 2014, Janelia Farm, in Ashburn, Virginia.
    
  • [Kavli Salon: Neurodata Without Borders 1](https://www.nwb.org/event/kavli-salon-neurodata-without-borders-1/): February 16, 2014, Hyatt Regency Chicago
    

Otherwise this looks good to me.

Copy link
Contributor

@stephprince stephprince left a comment

Choose a reason for hiding this comment

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

this all looks great to me!

@bendichter bendichter merged commit ce3dc66 into main Mar 22, 2025
4 of 5 checks passed
@bendichter bendichter deleted the add-events2 branch March 22, 2025 05:51
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.

4 participants