satRdays / hugo-satrdays-theme

theme for satRdays conference websites
Apache License 2.0
8 stars 7 forks source link

Merge speaker talk data file #39

Closed RMHogervorst closed 4 years ago

RMHogervorst commented 4 years ago

Fix #7 and probably #28 too.

RMHogervorst commented 4 years ago

Modify the modal to also include bio of speaker and uses the profile picture: Screenshot 2020-10-20 at 08 31 16 Talks part still works, tweetthis partial includes name of speaker (easy because it is in the same file) Screenshot 2020-10-20 at 08 30 35

Screenshot 2020-10-20 at 08 30 52

RMHogervorst commented 4 years ago

Summarizing:

This PR:

DaveParr commented 4 years ago

This looks really great, and thanks for the rebase, you'd read my mind. I've decided (partly thanks to all your work this month) that we need to have a develop branch as the PR target.

I've changed the target of this PR to that manually, I think because of your rebase that means everything is as intended.

I'm very happy to merge this in right now, and it does close #7 . It also almsot completely closes #28 with one exception:

The twitter user is not tagged by their handle (e.g. @sometwitterthing ) in the intent. I think this should be doable with a 'small' modification:

Currently the social section of what is now the data/schedule/talkx.toml like it does in this file from Columbus2020. The key/awakward part is that it expects a full string such as "https://twitter.com/Awong234". Would it be possible to extend the tweetthis.html partial with some form of string parsing, so it extracts only the handle from the url?

The double awkward bit would be the logic of putting in only the speakers 'listed' name if that twitter handle value was not present.

If you're interested in having a crack at that it would be awesome. As it is right now though I'd gratefully merge this in as is if you'd rather.

RMHogervorst commented 4 years ago

Let me check tomorrow

RMHogervorst commented 4 years ago

I think that string parsing thing should to be a separate issue. Let me check if I can fix it up today.

DaveParr commented 4 years ago

I'm going ahead to merge this. We can tackle that twitter handle string parsing problem in another branch.

Thanks a tonne for this :)