feat: add package of text-selector #171

v61265 · 2023-05-01T18:03:29Z

描述

隨機文字選擇器。
按照 jsonUrls 順序載入，並點擊按鈕隨機 heightlight 不同 object，剩下兩則是載入下一個 JSON 檔，若都載完則回到第一筆資料。

Demo

搭配 ecg package 後可放置於 cms 上

待優化處

目前預設的 props 都隨便抓，像是圖片之類的感覺可以放在 package assets 中。
第一次載入後，就算 mounted 結束， EmpasizedCircle 也抓不到新的 translateToParagraph。
有夠濫用 useRef，應該要清一清 QQ

nickhsine · 2023-05-02T05:01:17Z

packages/text-selector/package.json

+    "react-dom": "^18.1.0",
+    "serialize-javascript": "^6.0.0",
+    "styled-components": "5.3.5",
+    "uuid": "^8.3.2"


dependency 需要調整一下。
可以參考其他 pkg 的寫法。

已於 2abcaad 修改。

nitpick:
styled-components 忘記移除了？

question:
uuid 是忘記移除嗎？
還是我們程式碼哪裡有用到 uuid ？

感謝提醒，已於 994c2c7 修改。

nickhsine · 2023-05-02T05:02:49Z

packages/text-selector/yarn.lock

@@ -0,0 +1,2281 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.


text-selector 屬於 monorepo 的管理範圍，所以我們只需要保留 root 的 yarn.lock，不需要有 subpkg 的 yarn.lock。

已於 2abcaad 修改，感謝提醒！

v61265 · 2023-05-02T08:40:27Z

想請教 @nickhsine 一個 bug：
這是目前包出來的 embed code 放在 readr 文章中的 demo：
https://v3-dev.readr.tw/post/2340
目前看起來第一次的圈圈會稍微無法對齊，點擊下一則後又可以抓到正確位移資料，不過 localhost 無法重現該問題。

nickhsine · 2023-05-03T05:04:33Z

package.json

@@ -12,6 +12,7 @@
    "packages/share-button",
    "packages/three-story-points",
    "packages/dual-slides",
+    "packages/text-selector"


少了逗點

請問是指後面的逗號嗎？我的 Prettier 好像會自動拿掉，這個會有影響嗎？

沒事，不用理會這個訊息。目前的版本是對的。

nickhsine · 2023-05-04T02:01:44Z

package.json

@@ -12,6 +12,7 @@
    "packages/share-button",
    "packages/three-story-points",
    "packages/dual-slides",
+    "packages/text-selector"


沒事，不用理會這個訊息。目前的版本是對的。

nickhsine · 2023-05-04T02:02:13Z

packages/text-selector/README.md

@@ -0,0 +1,46 @@
+# [@readr-media/text-selector](https://www.npmjs.com/package/@readr-media/text-selector) &middot; ![npm version](https://img.shields.io/npm/v/@readr-media/react-karaoke.svg?style=flat)


question:
是不是要 follow convention，把 text-selector 改成 react-text-selector 呢？

另外，有空的話，是不是補個 demo 的影片／gif 呢？

好哇，是不是可以順便補上 props 建議使用方法，還是 jsDoc 都會 cover 這部分？

好像可以補上怎麼使用 TextSelector component ？

已於 b840ddc 修改，再麻煩幫忙看看，感謝！

是想說要不要統一 pkg 命名的方式，在前方加上 react- prefix？

nickhsine · 2023-05-04T02:03:20Z

packages/text-selector/package.json

+    "react-dom": "^18.1.0",
+    "serialize-javascript": "^6.0.0",
+    "styled-components": "5.3.5",
+    "uuid": "^8.3.2"


nitpick:
styled-components 忘記移除了？

nickhsine · 2023-05-04T02:05:52Z

packages/text-selector/src/react-components/hooks/useViewport.js

+      })
+    }
+    // Add event listener
+    window.addEventListener('resize', handleResize)


suggestion:
handleResize 看要不要用 throttle 降低被觸發的頻率？

對齁，感謝提醒，已於 6f8711b 修改。

nickhsine · 2023-05-04T02:06:23Z

yarn.lock

+  version "1.1.5"
+  resolved "https://registry.yarnpkg.com/default-import/-/default-import-1.1.5.tgz#6ba81645f79fa2e122bbac09651f30b55a8d9d5d"
+  integrity sha512-aaJ6uzZlmaEcN1U8yvtiyV7MG3/zZQf1XtGSW5dTfAVvfk0VZuriJelXxVL9a0ni42vMkhjWcztFfpIhwFcfOQ==
+


nitpick:
麻煩確認一下 default-import 是哪裡來的？

剛剛看好像是這個的？
https://github.com/readr-media/react/blob/main/packages/dual-slides/package.json#L31

那我再去 dual-slides 把 default-import 移除～

updated:
已經移除 default-import 囉
#195

nickhsine · 2023-05-04T02:12:09Z

packages/text-selector/src/react-components/components/debugger.js

+        </ControllerBtn>
+      )}
+      {jsonFileIndex < jsonLength - 1 && (
+        <ControllerBtn onClick={() => setJsonFileIndex((prev) => prev + 1)}>


question:
可否直接寫成 setJsonFileIndex(prev+1) 就好？
想請問這邊用 updater function 有什麼特殊用意嗎？

這裡應該沒有 prev 的 variable，所以是指直接寫 setJsonFileIndex(jsonFileIndex+1) 嗎？
我自己的理解是因為 useState 是非同步的，為了防止真的重新更新 state 之前，使用者又重複點擊，才會用 function 確保真的是拿到已經改變過後的值。

這邊應該是寫 setJsonFileIndex(jsonFileIndex+1) 就好？
如果你想要在一個 click 中，連續 +1 兩次的話，
就要改用 setJsonFileIndex((prev) => prev + 1)，
但是我們目前應該沒有這個需求。

了解，感謝說明，已於 6f8711b 修改。

nickhsine · 2023-05-04T02:22:20Z

packages/text-selector/src/react-components/index.js

+/**
+ *  @param {Object} opts
+ *  @param {string} [opts.className]
+ *  @param {string[]} [opts.jsonUrls]


nitpick:
有default value 的話，麻煩也加在 JSDoc 中

感謝提醒，已於 a8a8c43 修改。

nickhsine

question:
目前 PR 上的程式碼，好像 resize 後，畫面會跑掉？

nickhsine · 2023-05-04T02:24:02Z

packages/text-selector/src/react-components/index.js

+  const renderedData = useMemo(() => {
+    const returnArr = []
+    data.forEach((item) => {
+      console.log(item)


nitpick:
console.log 記得移除？

感謝提醒，已於 994c2c7 修改。

nickhsine · 2023-05-04T02:34:55Z

packages/text-selector/src/react-components/index.js

+  width: calc(100vw - 40px);
+  padding-inline-start: 20px;
+  max-width: 712px;
+  font-family: 'Noto Sans TC';


suggestion:
字型可以考慮使用 inherit ，讓使用 embed code 的 client 決定用什麼字型

感謝提醒，已於 05842ac 修改。

nickhsine · 2023-05-04T08:58:50Z

packages/text-selector/src/react-components/index.js

+  // mounted 後，才變動 highlightIndex
+  useEffect(() => {
+    setHighlightIndex(0)
+  }, [])


question:
想知道為何要 mounted 後才把 highlightIndex 設定成 0？

因為遇到太多 render 完後第一則的位置跑掉的問題，想說這樣改會不會一勞永逸或多加一層保險（？）不過確實不太直覺？

nickhsine · 2023-05-04T09:00:40Z

packages/text-selector/src/react-components/index.js

+    setHighlightIndex((prev) => prev + 1)
+    if (highlightIndex > dataLength - 2) {
+      setHighlightIndex(1)
+    }


question:
這邊邏輯是不是要改成

if (highlightIndex > dataLength - 2) { setHighlightIndex(1) } else { setHighlightIndex(highlightIndex + 1) }

？

感謝提醒，已於 fdc370b 修正。

nickhsine · 2023-05-04T09:07:53Z

packages/text-selector/src/react-components/index.js

+          return {
+            content: item,
+            order,
+          }


nitpick:
fetchData function 這段可以加註解嗎？
有點不是很懂資料處理的邏輯。

確實有點複雜，我先在 e837d5e 試寫一版，再麻煩幫忙看看，感謝🙏

… structure

v61265 · 2023-05-04T17:53:30Z

question:
目前 PR 上的程式碼，好像 resize 後，畫面會跑掉？

這部分是參考 nick 之前 tsp 改寫的，目前加上 width 作為 dependency，好像還是沒有很準確，有時候甚至會出現負值，想請問可能原因🙋‍♀️

2023/5/10 更新
因為考量需求和實作成本，已於 814100e 將 shiftLeft 和 /hooks/usViewports 取消，交給。若需要移動至畫面左邊，請使用者額外處理。

nickhsine · 2023-05-10T05:53:20Z

packages/text-selector/src/react-components/hooks/useViewport.js

+        nowHeight < prevHeight * 0.6 ||
+        nowHeight === prevHeight
+      ) {
+        prevHeight = window.innerHeight


question:
prevHeight 和 windowSize.height 有什麼不同嗎？

nickhsine · 2023-05-10T05:54:21Z

packages/text-selector/src/react-components/hooks/useViewport.js

+      if (
+        nowHeight > prevHeight * 1.4 ||
+        nowHeight < prevHeight * 0.6 ||
+        nowHeight === prevHeight


question:
為何要特別判斷 nowHeight === prevHeight？
如果兩個相等，不是代表不需要處理 resize 嗎？

因為 resize 是偵測長寬，如果等於的話就表示其實變動的是螢幕寬，因此還是要處理。
關於偵測螢幕尺寸的 hooks，因為有一些預料之外的行為（像是 safari 和 in-app 的工具列出現離開都會導致畫面閃現），考慮後已於 814100e 拔除。

nickhsine

LGTM 👍

BTW, 如果可以的話，請 build 出 cjs 和 esm 的版本。
可以參考 0d2f4eb

feat: add package of text-selector

1ae7c66

v61265 requested a review from nickhsine May 1, 2023 18:03

nickhsine reviewed May 2, 2023

View reviewed changes

v61265 added 3 commits May 2, 2023 14:21

refactor: adjust json data scruture

e351bb3

fix: debug circle position of first time

8e69fb8

chore: adjust package.json

2abcaad

v61265 requested a review from nickhsine May 2, 2023 08:08

refactor: adjust wrong spelling

b95b513

nickhsine reviewed May 3, 2023

View reviewed changes

fix(rtx): getTranslateToParagraph after rendered

c318c01

v61265 force-pushed the text branch from 5436a80 to c318c01 Compare May 3, 2023 06:02

fix: debugger mode scroll culculator

af64154

nickhsine reviewed May 4, 2023

View reviewed changes

v61265 added 2 commits May 4, 2023 15:25

refactor: remove console.log

994c2c7

chore: remove styled-components and uuid of dependencies

cc13542

nickhsine reviewed May 4, 2023

View reviewed changes

v61265 added 4 commits May 4, 2023 18:06

refactor: add throttle to handleResized

6f8711b

style: set font-family inherit

05842ac

refactor: refactor function handleOnClickBtn

fdc370b

refactor: add comment of fetchData and refactor handleChangeFileIndex…

e837d5e

… structure

v61265 added 4 commits May 5, 2023 02:24

refactor: prevent trigger resize when height not change more than 40%

dfa2d1f

refactor: remove shift left and hooks/useViewports

814100e

refactor: add default value of JSDoc

a8a8c43

chore: upgrade to 1.1.1-beta.7

db65c3d

v61265 added 3 commits May 5, 2023 16:16

style: adjust margin

0a04ee8

style: adjust mobile circle and nextBtn

c71e62a

refactor: store a viewport in state

1bfadbe

nickhsine reviewed May 10, 2023

View reviewed changes

v61265 and others added 2 commits May 10, 2023 22:32

chore: change name to random-text-selector and add readme demo & usage

b840ddc

Merge branch 'main' into text

f5cb1d7

v61265 requested a review from nickhsine May 10, 2023 14:52

nickhsine reviewed May 11, 2023

View reviewed changes

v61265 added 2 commits May 11, 2023 15:57

chore: change pkg name to react-random-text-selector

c00ef46

refactor: build cjs & esm version

5baa331

v61265 merged commit cf8c92e into readr-media:main May 11, 2023

		@@ -0,0 +1,2281 @@
		# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.

		@@ -0,0 +1,46 @@
		# [@readr-media/text-selector](https://www.npmjs.com/package/@readr-media/text-selector) · ![npm version](https://img.shields.io/npm/v/@readr-media/react-karaoke.svg?style=flat)

feat: add package of text-selector #171

feat: add package of text-selector #171

Conversation

v61265 commented May 1, 2023 • edited Loading

描述

Demo

待優化處

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

v61265 commented May 2, 2023

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

v61265 May 10, 2023 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickhsine May 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickhsine May 4, 2023 • edited Loading

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

nickhsine 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

v61265 May 4, 2023 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

v61265 commented May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickhsine left a comment

Choose a reason for hiding this comment

v61265 commented May 1, 2023 •

edited

Loading

v61265 May 10, 2023 •

edited

Loading

nickhsine May 11, 2023 •

edited

Loading

nickhsine May 4, 2023 •

edited

Loading

v61265 May 4, 2023 •

edited

Loading

v61265 commented May 4, 2023 •

edited

Loading