Skip to content

Conversation

chohongm
Copy link
Contributor

@chohongm chohongm commented Aug 27, 2024

Fixes CLNP-4852

Changelogs

  • Will be added soon.

How to test?

그룹채널에 유저 두명:
test_liam_1, test_liam_2 (닉네임도 유저id와 동일 함)

로컬 url:
http://localhost:5173/group_channel?appId=5D27A98C-D935-4EDA-846A-BCCD90E8E55B&userId=test_liam_1&nickname=test_liam_1&accessToken=3810634cd02e61956f08edf4fd614bfbe9e3e505

대시보드:
https://dashboard.sendbird.com/5D27A98C-D935-4EDA-846A-BCCD90E8E55B/settings/general

Figma

https://www.figma.com/design/SVbXU00FhjztekD8AiVukK/WIP_UIKit_React?node-id=2924-4031&m=dev

Screenshot 2024-09-06 at 2 13 32 PM

Default container type rendering (template message only)

WIP_UIKit_React.Figma.webm

@chohongm chohongm added the DO_NOT_MERGE Do not merge this PR yet label Aug 27, 2024
@chohongm chohongm self-assigned this Aug 27, 2024
Copy link

netlify bot commented Aug 27, 2024

Deploy Preview for sendbird-uikit-react ready!

Name Link
🔨 Latest commit 8b20f7b
🔍 Latest deploy log https://app.netlify.com/sites/sendbird-uikit-react/deploys/66f4f1debf47f2000854ab9e
😎 Deploy Preview https://deploy-preview-1207--sendbird-uikit-react.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.

@bang9
Copy link
Contributor

bang9 commented Aug 28, 2024

혹시 메세지 보내져있는 채널에 로그인 할 수 있는 정보 하나만 알려주실 수 있을까용!

@chohongm
Copy link
Contributor Author

@bang9 어제까지만해도 아래걸로 테스트했었는데 오늘은 안되네요..?
http://localhost:5173/group_channel?appId=6b9b0bdb-2761-4316-850c-118a518cc2e5&userId=test-liam-01&nickname=test-liam-01

@chohongm chohongm marked this pull request as draft August 29, 2024 01:17
@chohongm chohongm force-pushed the feat/CLNP-4852-improve-message-template-view branch from 73913bf to a52d9f1 Compare August 30, 2024 08:41
@chohongm chohongm requested a review from bang9 August 30, 2024 08:55
@chohongm chohongm marked this pull request as ready for review August 30, 2024 08:55
@chohongm chohongm removed the DO_NOT_MERGE Do not merge this PR yet label Aug 30, 2024
@bang9
Copy link
Contributor

bang9 commented Aug 31, 2024

변경된 carousel template 구조로는 https://github.com/sendbird/sendbird-uikit-core-ts/pull/150 이 브랜치에서
react 용 메세지 템플릿 패키지를 빌드하고 resolutions 로 연결하여 테스트 하고 계시면 될 것 같습니다.

캐러셀이 아직 버그가 수정은 안돼서 렌더링은 되지만 동작은 안할텐데요
UIKit 에서는 변경된 구조의 Carousel syntax 반영을 하는 목적으로 먼저 진행 부탁드리겠습니다.

[중요]

  • 그리고 argb 를 rgba 로 처리하는 코드도 삭제하셔야 할거에요!
  • 템플릿 데이터의 string 을 number 로 변환하는 코드가 있다면, 이것도 제거해도 됩니다.

@bang9 bang9 changed the title feat: Improve message template view (draft) feat: Improve message template view Sep 2, 2024
@chohongm chohongm requested a review from bang9 September 24, 2024 06:05
chohongm and others added 3 commits September 24, 2024 15:47
Co-authored-by: Hyungu Kang | Airen <gusrn1423@naver.com>
@chohongm chohongm requested a review from bang9 September 25, 2024 07:52
};

const totalBottom = useMemo(() => getTotalBottom(), [isTimestampBottom]);
const totalBottom = useMemo(() => getTotalBottom(), []);
Copy link
Contributor

Choose a reason for hiding this comment

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

이 totalBottom 도, wide 같은 layout 때문에 추가되었던 것 같은데.. 혹시 이전 상태 기억 나시나요?
돌리는게 좋을까요, 아니면 일단 두는게 좋을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

요건 없애는게 맞습니다. container type 이 default 가 아닐 때 bottom 값을 지정해두는 로직이었습니다. 이제는 없어졌기 때문에 위 로직도 같이 없애주는게 맞습니다.

export enum UI_CONTAINER_TYPES {
  DEFAULT = '',
  WIDE = 'ui_container_type__wide',
  DEFAULT_CAROUSEL = 'ui_container_type__default-carousel',
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@liamcho 요고는 티켓 만들어서 진행해주시면 감사하겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

두는게 좋아보입니다. useMemo() 는 없애야 할것 같습니다. 있으면 실시간 업뎃이 안되네요. profile 의 bottom 위치를 메세지에 feedback button 들이 있을때, 또 thread replies 가 있을 때 계산하는 로직이라 필요합니다. 이전 로직은

.sendbird-message-content__left__avatar.use-thread-replies {
      bottom: 35px;
    }

를 사용했는데 extensible 하지 않고 constant 값을 사용해서 별로 좋은 방법 같지는 않습니다.

chohongm and others added 5 commits September 26, 2024 13:48
Co-authored-by: Hyungu Kang | Airen <gusrn1423@naver.com>
Co-authored-by: Hyungu Kang | Airen <gusrn1423@naver.com>
@chohongm chohongm requested a review from bang9 September 26, 2024 04:59
@chohongm chohongm force-pushed the feat/CLNP-4852-improve-message-template-view branch from 7e01cd9 to 8b20f7b Compare September 26, 2024 05:32
Copy link
Contributor

@bang9 bang9 left a comment

Choose a reason for hiding this comment

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

lgtm!

@chohongm chohongm merged commit 6fb1baa into main Sep 26, 2024
10 checks passed
@chohongm chohongm deleted the feat/CLNP-4852-improve-message-template-view branch September 26, 2024 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants