-
Notifications
You must be signed in to change notification settings - Fork 1
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: link pr in the env view #540
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the current behavior isn't ideal for user experience. From a user's perspective, clicking on the branch name should navigate directly to the branch details. One potential solution might be to introduce an additional tag showing only the PR number (e.g., #10254
). However, in any case, we should also include a hover effect to clarify that the element is clickable and redirects the user elsewhere.
What do you think about that? I know there are already a lot of elements in this view, but I believe this approach would provide a better overall user experience.
30d792b
to
3284f38
Compare
@egekocabas added a separate tag for PR and disabled navigation for release tags, Paul has implemented the release sync from github and we can implement a link to the release once his pr is merged. Screen.Recording.2025-03-16.at.17.23.40.mov |
That looks great thanks for adding separate link for PRs! Maybe a minor change: I think we don't need to write PR as a prefix since this is a known pattern for developers -> #<PR_NUMBER>. Also we have the icon as well :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice and clean! I just have a minor change, that we should address :) 🥳
client/src/app/components/environments/environment-list/environment-list-view.component.html
Show resolved
Hide resolved
client/src/app/components/environments/environment-list/environment-list-view.component.html
Show resolved
Hide resolved
edeb523
to
516c4da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Great job, thanks for adding this feature 🚀 🎊
I didn't want to overload the environment list component even more that's why added a link to the existing latest deployment tag.
Screen.Recording.2025-03-15.at.17.29.34.mov
fixes #317