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

Show label on the app detail and list page #3011

Merged
merged 5 commits into from
Jan 12, 2022

Conversation

nakabonne
Copy link
Member

@nakabonne nakabonne commented Jan 7, 2022

What this PR does / why we need it:
With it we can see labels on the application detail page as well as the application list page.

On the detail page:
image

On the list:
image

Which issue(s) this PR fixes:

Fixes #3009

Does this PR introduce a user-facing change?:

NONE

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 31.57%. This pull request does not change code coverage.

@nakabonne
Copy link
Member Author

/trigger web-test

@pipecd-bot
Copy link
Collaborator

TRIGGER

@nakabonne: Your requested web-test has been scheduled in response to this comment.

@nakabonne
Copy link
Member Author

/trigger web-test

@pipecd-bot
Copy link
Collaborator

TRIGGER

@nakabonne: Your requested web-test has been scheduled in response to this comment.

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 81.87%. This pull request decreases coverage by -0.02%.

File Base Head Diff
src/components/application-detail-page/application-detail/index.tsx 96.55% 94.92% -1.64%

@nakabonne nakabonne changed the title Show label chips on the app detail page Show label on the app detail and list page Jan 7, 2022
@nakabonne
Copy link
Member Author

Updated to show them on the application list page as well.

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 31.57%. This pull request does not change code coverage.

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 81.85%. This pull request decreases coverage by -0.04%.

File Base Head Diff
src/components/application-detail-page/application-detail/index.tsx 96.55% 94.92% -1.64%
src/components/applications-page/application-list/application-list-item/index.tsx 93.62% 91.67% -1.95%

@nakabonne
Copy link
Member Author

Since we're looking into updating the Environment when adding a label that has env as a key
/hold

@nakabonne
Copy link
Member Author

/hold

@nakabonne
Copy link
Member Author

/rebase

@pipecd-bot
Copy link
Collaborator

REBASE

@nakabonne: Rebased this pull request in response to this comment.

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 31.57%. This pull request does not change code coverage.

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 81.82%. This pull request decreases coverage by -0.04%.

File Base Head Diff
src/components/application-detail-page/application-detail/index.tsx 96.55% 94.92% -1.64%
src/components/applications-page/application-list/application-list-item/index.tsx 93.62% 91.67% -1.95%

@pipecd-bot pipecd-bot added size/M and removed size/S labels Jan 12, 2022
@nakabonne
Copy link
Member Author

/hold cancel

@nakabonne
Copy link
Member Author

The warning icon and header text couldn't be aligned. Let me correct it later.
image

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 31.57%. This pull request does not change code coverage.

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 81.83%. This pull request decreases coverage by -0.04%.

File Base Head Diff
src/components/application-detail-page/application-detail/index.tsx 96.55% 94.92% -1.64%
src/components/applications-page/application-list/index.tsx 90.57% 90.74% +0.17%
src/components/applications-page/application-list/application-list-item/index.tsx 93.62% 91.67% -1.95%

@@ -217,6 +221,14 @@ export const ApplicationDetail: FC<ApplicationDetailProps> = memo(
{env.name}
</Typography>
)}
{app?.labelsMap.map((label, i) => (
<Chip
label={label[0] + ":" + label[1]}
Copy link
Member

Choose a reason for hiding this comment

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

How about adding a space between the value and key as key: value?

Copy link
Member Author

Choose a reason for hiding this comment

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

For the app list page, you can't do that because the line breaks make it difficult to read. But for the detail page, looks nice:
image

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied!

Copy link
Member

Choose a reason for hiding this comment

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

Thank you. 👍

@nghialv
Copy link
Member

nghialv commented Jan 12, 2022

Can't wait to see it. Great job!
/lgtm

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 31.57%. This pull request does not change code coverage.

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 81.83%. This pull request decreases coverage by -0.04%.

File Base Head Diff
src/components/application-detail-page/application-detail/index.tsx 96.55% 94.92% -1.64%
src/components/applications-page/application-list/index.tsx 90.57% 90.74% +0.17%
src/components/applications-page/application-list/application-list-item/index.tsx 93.62% 91.67% -1.95%

…index.tsx

Co-authored-by: Le Van Nghia <nghialv2607@gmail.com>
@pipecd-bot pipecd-bot removed the lgtm label Jan 12, 2022
@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 31.57%. This pull request does not change code coverage.

@nghialv
Copy link
Member

nghialv commented Jan 12, 2022

/lgtm

@khanhtc1202
Copy link
Member

/trigger web-test

@pipecd-bot
Copy link
Collaborator

TRIGGER

@khanhtc1202: Your requested web-test has been scheduled in response to this comment.

@khanhtc1202
Copy link
Member

/approve

@pipecd-bot
Copy link
Collaborator

APPROVE

This pull request is APPROVED by khanhtc1202.

Approvers can cancel the approval by writing /approve cancel in a comment. Any additional commits also will change this pull request to be not-approved.

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 81.83%. This pull request decreases coverage by -0.04%.

File Base Head Diff
src/components/application-detail-page/application-detail/index.tsx 96.55% 94.92% -1.64%
src/components/applications-page/application-list/index.tsx 90.57% 90.74% +0.17%
src/components/applications-page/application-list/application-list-item/index.tsx 93.62% 91.67% -1.95%

@khanhtc1202
Copy link
Member

/trigger web-test

@pipecd-bot
Copy link
Collaborator

TRIGGER

@khanhtc1202: Your requested web-test has been scheduled in response to this comment.

@pipecd-bot pipecd-bot merged commit 8abbf5a into master Jan 12, 2022
@pipecd-bot pipecd-bot deleted the show-labels-on-app-detail branch January 12, 2022 11:16
@nghialv
Copy link
Member

nghialv commented Jan 13, 2022

/changelog

@pipecd-bot
Copy link
Collaborator

CHANGELOG

@nghialv: Changelog has been generated in response to this comment.

Details

Changelog since v0.23.0

Notable Changes

  • The application description is now changeable via your application configuration file (#3017)
  • Fix a bug that the deployment pipeline gets invisible when it has a long sentence. (#3013)
  • Add a More button to fetch older deployments (#2999)
  • Filtering deployments by Labels is now available (#3000)
  • Filtering applications by Labels is now available (#2987)

Internal Changes

  • Show label on the app detail and list page (#3011)
  • Put deprecated notes on the Environment tab (#3022)
  • Remove duplicate value in version details on application list page (#3021)
  • Simplify Piped installation flow in the quickstart guide (#3020)
  • Fix diff description and image (#3018)
  • Enable to register Environment as a label (#3016)
  • Suggest possible labels on the apps/deploys filter (#3012)
  • Use filestore as datastore RFC (#2959)
  • Add monthly update for January 2022 (#3010)
  • Change text shown on deployment/application labels filter (#3005)
  • Show labels specified in the query parameter (#3003)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show the labels on the application pages
4 participants