Skip to content

[WIP] Add ConsoleNotification CRD that can appear above, below, or above and below the page with option to dismiss #1325

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.

Sign up for GitHub

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

Jump to bottom
Closed
rhamilto wants to merge 1 commit into from
Closed

[WIP] Add ConsoleNotification CRD that can appear above, below, or above and below the page with option to dismiss #1325

rhamilto wants to merge 1 commit into from

Conversation

@rhamilto
Copy link
Member

@rhamilto rhamilto commented Mar 22, 2019 •
edited
Loading

Differs from #1542 in that there is a dismiss option.

TODOS:

  • Remove extensions/console-notification.crd.yaml from PR as it will live in the console operator

localhost_9000_k8s_cluster_console openshift io_v1_ConsoleNotification_example_yaml (1)

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 22, 2019
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 22, 2019
@mbach04
Copy link

mbach04 commented Mar 25, 2019

Will the notification background color be configurable as well? The classification banner looks great, but it's probably best to not keep the standard green success banner notification directly under it as green generally denotes Unclassified content. If that color could be made blue like the oauth message, it wouldn't conflict with any network banner color.

@spadgett
Copy link
Member

spadgett commented Mar 25, 2019

Hey @mbach04, the green banner is another custom message with different options. It's not a standard console notification.

Background color is configurable.

This was referenced Mar 29, 2019
Closed
Closed
@rhamilto rhamilto force-pushed the console-notification-crd-top-bottom-dismissible branch 5 times, most recently from 91c2af2 to d7dff1d Compare March 29, 2019 17:20
@rhamilto rhamilto changed the base branch from master to master-next March 29, 2019 17:25
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 29, 2019
@rhamilto rhamilto force-pushed the console-notification-crd-top-bottom-dismissible branch from d7dff1d to 4569aa1 Compare March 29, 2019 17:49
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 29, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 29, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhamilto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhamilto rhamilto force-pushed the console-notification-crd-top-bottom-dismissible branch 2 times, most recently from c1c7cdc to 1f1dcee Compare April 2, 2019 14:05
@spadgett spadgett added this to the v4.2 milestone Apr 7, 2019
@rhamilto rhamilto force-pushed the console-notification-crd-top-bottom-dismissible branch 2 times, most recently from 912c856 to 798eb0e Compare April 8, 2019 13:37
@rhamilto rhamilto force-pushed the console-notification-crd-top-bottom-dismissible branch 3 times, most recently from 9c494d8 to 35804c4 Compare April 18, 2019 13:48
@rhamilto rhamilto force-pushed the console-notification-crd-top-bottom-dismissible branch from 35804c4 to a21b247 Compare April 23, 2019 17:22
@rhamilto rhamilto force-pushed the console-notification-crd-top-bottom-dismissible branch 2 times, most recently from 6907ff8 to 4d1df51 Compare May 6, 2019 12:38
@rhamilto rhamilto mentioned this pull request May 7, 2019
Merged
@rhamilto rhamilto changed the title [WIP] Add ConsoleNotification CRD that can appear above or below the page with option to dismiss [WIP] Add ConsoleNotification CRD that can appear above, below, or above and below the page with option to dismiss May 7, 2019
@rhamilto rhamilto force-pushed the console-notification-crd-top-bottom-dismissible branch from 4d1df51 to e6c6468 Compare May 7, 2019 15:21
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2019
@rhamilto rhamilto force-pushed the console-notification-crd-top-bottom-dismissible branch from e6c6468 to ebb485d Compare May 13, 2019 14:09
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2019
@spadgett spadgett changed the base branch from master-next to master May 13, 2019 22:51
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 13, 2019

@rhamilto: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/frontend ebb485d link /test frontend

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@spadgett spadgett added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label May 14, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 14, 2019

@rhamilto: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2019
rhamilto
rhamilto commented May 15, 2019
View reviewed changes
extensions/console-notification.crd.yaml
type: string
description: Absolute URL for the link
pattern: '^http(s)?://([\w-]+.)+[\w-]+(/[\w- ./?%&=])?$'
opensNewWindow:
Copy link
Member Author

@rhamilto rhamilto May 15, 2019

Choose a reason for hiding this comment

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

Let's remove this and make these always open in a new window.

rhamilto
rhamilto commented May 15, 2019
View reviewed changes
frontend/public/components/console-notifier.tsx
<p className="co-global-notification__text">
{notification.spec.text} {_.get(notification.spec, ['link', 'href'])
&& <a href={notification.spec.link.href}
target={notification.spec.link.opensNewWindow ? '_blank' : null}
Copy link
Member Author

@rhamilto rhamilto May 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Add rel="noopener nofollow"

@rhamilto
Copy link
Member Author

rhamilto commented May 15, 2019

Closing in favor of #1542

@rhamilto rhamilto closed this May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhadvig jhadvig Awaiting requested review from jhadvig

@sg00dwin sg00dwin Awaiting requested review from sg00dwin

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

v4.2

Development

Successfully merging this pull request may close these issues.

4 participants

@rhamilto @mbach04 @spadgett @openshift-ci-robot