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(3277): Support banner display in the UI pipeline landing page #1326

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sagar1312
Copy link
Member

Context

Banner messages can now be created for specific pipeline.
Such banners are not currently rendered in the UI.

image

Objective

Fetch and render pipeline specific banners messages in pipeline pages.

image

References

screwdriver-cd/screwdriver#3277

License

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@sagar1312 sagar1312 force-pushed the sagar1312-feat-3277-pipeline_banners branch 3 times, most recently from fafba75 to e4b6e4c Compare February 3, 2025 18:46
@sagar1312 sagar1312 force-pushed the sagar1312-feat-3277-pipeline_banners branch from e4b6e4c to a750699 Compare February 3, 2025 18:57
@sagar1312 sagar1312 force-pushed the sagar1312-feat-3277-pipeline_banners branch from a750699 to 7efea61 Compare February 3, 2025 19:00
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a Glimmer component for the banner

@@ -0,0 +1,20 @@
{{#each this.banners as |banner|}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{{#each this.banners as |banner|}}
{{#each @banners as |banner|}}

Copy link
Contributor

Choose a reason for hiding this comment

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

You'll need to modify the stylesheet to use SASS syntax for the variables and to properly export the style sheet to be included

@@ -36,6 +36,15 @@ export default class PipelineOptionsRoute extends Route {
return [];
});

return { pipeline, jobs };
const banners = await this.shuttle
Copy link
Contributor

Choose a reason for hiding this comment

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

Banners shouldn't be fetched in this route for the V2 UI

@@ -1,9 +1,9 @@
& {
.banner .alert {
height: $nav-banner-height;
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to readjust the spacing allocated to the banners in the CSS variables since having multiple banners in the current implementation will break the page experience forcing a vertical scroll on the page.

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