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

Frame, member and project detail layout #67

Open
wants to merge 2 commits into
base: v3
Choose a base branch
from
Open

Frame, member and project detail layout #67

wants to merge 2 commits into from

Conversation

Soecka
Copy link

@Soecka Soecka commented Oct 26, 2024

PR-67 PR-67 PR-67 Powered by Pull Request Badge

Co-authored-by: South Drifted [email protected]

Checklist(清单):

  • Labels
  • Assignees
  • Reviewers

[add] i18n switch, LarkImage component
[polish] project, member detail pages
@Soecka Soecka added the enhancement Some improvements label Oct 26, 2024
@Soecka Soecka added this to the 官网3.0 milestone Oct 26, 2024
@Soecka Soecka requested a review from TechQuery October 26, 2024 07:39
components/Project/Card.tsx Outdated Show resolved Hide resolved
components/ScrollList.tsx Show resolved Hide resolved
pages/index.tsx Show resolved Hide resolved
Comment on lines -22 to +27
<div className="container mx-auto max-w-screen-xl">
<PageHead title={t('member')} />

<h1 className="my-8">{t('member')}</h1>

<ScrollList
translator={i18n}
store={memberStore}
renderList={allItems => <MemberListLayout defaultData={allItems} />}
defaultData={list}
/>
</div>
<Frame
title={t('member')}
header={t('member')}
store={memberStore}
Layout={MemberListLayout}
defaultData={list}
/>
Copy link
Member

Choose a reason for hiding this comment

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

没必要这么高度封装吧?原有三者没有必然强关联,<Frame /> 的命名也应该只包含容器、标题,而非具体内容。

Copy link
Author

Choose a reason for hiding this comment

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

三个都是 PageHeadh1ScrollList 的结构,我可以更改命名为 FrameListLayout

Copy link
Member

Choose a reason for hiding this comment

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

三个都是 PageHeadh1ScrollList 的结构,我可以更改命名为 FrameListLayout

如果非要这样的话,命名应该是 ScrollListPage

export const blobURLOf = (value: TableCellValue) =>
value instanceof Array
? typeof value[0] === 'object' && ('file_token' in value[0] || 'attachmentToken' in value[0])
? `${fileBaseURI}/${value[0].name}`
Copy link
Member

Choose a reason for hiding this comment

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

不要再用文件名了,用 token。

Copy link
Author

Choose a reason for hiding this comment

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

R2 那个 PR 合了调通了再改,blobClient 这些都要改

Comment on lines +46 to 49
{/**
* @todo replace with LarkImage after R2 is ready
*/}
<img className="object-fill" src={fileURLOf(project.image)} alt={String(project.name)} />
Copy link
Member

Choose a reason for hiding this comment

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

现在可以直接用包装组件了,因为有自动的飞书后备接口。

Copy link
Author

Choose a reason for hiding this comment

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

R2 那个 PR 合了调通了再改,blobClient 这些都要改

Copy link
Member

Choose a reason for hiding this comment

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

R2 那个 PR 合了调通了再改,blobClient 这些都要改

包装组件也是调用的同样的 fileURLOf(),逻辑和路径拼接是解耦的,相互不影响。

pages/member/[nickname].tsx Outdated Show resolved Hide resolved
pages/member/[nickname].tsx Outdated Show resolved Hide resolved
pages/member/[nickname].tsx Show resolved Hide resolved
}

/**
* @todo remove ScrollList and use children instead?
Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Author

Choose a reason for hiding this comment

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

目前也是兼容直接传 children 的,先不做改动

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Some improvements
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants