Skip to content
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

Put keynote slides #946

Merged
merged 6 commits into from
Jan 11, 2021
Merged

Conversation

Maliaotw
Copy link
Contributor

Types of changes

  • Bugfix
  • New feature
  • Refactoring
  • Breaking change (any change that would cause existing functionality to not work as expected)
  • Documentation Update
  • Other (please describe)

Description

put keynote slides url to file

@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2020

Codecov Report

Merging #946 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #946   +/-   ##
=======================================
  Coverage   75.05%   75.05%           
=======================================
  Files          80       80           
  Lines        3083     3083           
=======================================
  Hits         2314     2314           
  Misses        769      769           

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 acec059...89a5396. Read the comment docs.

@yychen
Copy link
Member

yychen commented Sep 21, 2020

Wait wait... @Maliaotw 你只是把 json 加了這個欄位但是前端 template 好像不會秀阿 orz

@Maliaotw
Copy link
Contributor Author

Maliaotw commented Sep 24, 2020

修正了, 請再幫我看看

Copy link
Member

@yychen yychen left a comment

Choose a reason for hiding this comment

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

另外,就是你的 IDE 好像都會把整個縮排做處理,其實這樣就會很難看得出你到底實際上改的是哪些地方 orz

Comment on lines 80 to 82
<article data-target="tabbing.pane">
<a href="{{ data.slides }}">{{ data.slides }}</a>
</article>
Copy link
Member

Choose a reason for hiding this comment

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

你這樣的話,第一是那一整個 tab 會很空,第二個是沒有投影片的人這邊就會點下去是空白的...

我在想,你有沒有辦法把他簡單的做,加在 "演講" 的那個 tab 的內容裡,但是變成很像 social links 那樣,如果有投影片,就放在最下面,例如

  {% if data.slides %}
	 <div class="slides">
		  Slides: <a href="{{ data.slides }}" target="_blank" rel="noopener">{{ data.slides}}</a>
	 </div>
  {% endif %}

@tai271828
Copy link
Member

@Maliaotw is this merge request ready to review? If no, you may convert this pr to "draft" (this is a github feature). If yes, please signal us.

@mattwang44
Copy link
Member

@tai271828 Yes, this PR is done and is currently under review by me. However, since this PR (& also #782) is a part of the 2020 website, I personally wish Tom to be the final reviewer. It's my fault to make it being stale for so long since Tom & I haven't kicked-off the handover tasks until tomorrow (we'll have a meeting in person and go through the checklist for closing the 2020 website). So, please give us more time, and we'll close this PR soon 😅.

@tai271828
Copy link
Member

@mattwang44 no problem! I asked to make sure this PR is in people's radar, so we won't waste @Maliaotw 's contribution : )

Copy link
Member

@yychen yychen left a comment

Choose a reason for hiding this comment

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

ㄜ! 這跟我預期的不太一樣,但這樣子也是可以的啦 orz.

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.

None yet

6 participants