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

feat: add ReportDialog #1453

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

feat: add ReportDialog #1453

wants to merge 4 commits into from

Conversation

YongChenSu
Copy link
Collaborator

這個 PR 是?

  1. 公司/職稱頁面薪資列表,新增 UI。
  2. 單篇面試/工作心得頁面,新增 UI。

Screenshots

image

image

我應該從何看起?

先給出 UI,功能實作中,回報數量先給預設常數。

Questions:

串 API 中
已驗證,createSalaryWorkTimeReportcreateExperienceReportquery experience 功能正常。
但無法 query salaryWorkTime,有可能是 query 下錯,詢問中,討論串連結

我應該如何手動測試?

起 local

  1. 進入「公司/職稱頁面薪資列表」
  2. 進入「單篇面試/工作心得頁面」

@@ -110,3 +110,7 @@ export const formatWage = wage => {

export const formatDate = ({ year, month }) =>
`${year}.${month >= 10 ? '' : 0}${month}`;

export const getReportCount = (salaryWorkTimeReports = 0) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

從 callsite 來看 input 應該是 Array?

Suggested change
export const getReportCount = (salaryWorkTimeReports = 0) => {
export const getReportCount = (salaryWorkTimeReports = []) => {

@@ -70,6 +73,13 @@ const ReactionZone = ({ experienceId, onClickMsgButton }) => {
>
留言
</ReactionButton>
<ReactionButton className={styles.report} Icon={() => null}>
Copy link
Contributor

Choose a reason for hiding this comment

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

建議直接修改 ReactionButton 讓 Icon 為 optional

Suggested change
<ReactionButton className={styles.report} Icon={() => null}>
<ReactionButton className={styles.report}>

const expInYearText = formatExperienceInYear(experience.experience_in_year);
return (
<Fragment>
<div className={styles.reportDialogContainer}>
<ReportDialog reportCount={reportCount} />
{Boolean(reportCount) && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: 跟這個應該是一樣的意思

Suggested change
{Boolean(reportCount) && (
{reportCount && (

@barry800414
Copy link
Contributor

Hi @YongChenSu please help take a look at @peteranny 's comment.

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.

3 participants