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

Feature color cylc message bubbles #1436

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

markgrahamdawson
Copy link
Contributor

@markgrahamdawson markgrahamdawson commented Aug 15, 2023

Closes #1276

Moved the message chip into a new component
Added functionality to style based on props

Note - at the moment the 'level' isn't coming through from the api so chip color is decided based on message label.
This will need addressing in the future.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • no docs needed
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

CHANGES.md Outdated Show resolved Hide resolved
src/components/cylc/MessageChip.vue Show resolved Hide resolved
src/components/cylc/MessageChip.vue Outdated Show resolved Hide resolved
src/components/cylc/MessageChip.vue Outdated Show resolved Hide resolved
Comment on lines 45 to 49
['this is a debug message', ''],
['this is a info message', 'bg-grey'],
['this is a warning message', 'bg-warning'],
['this is an error message', 'bg-error'],
['this is a critical message', 'bg-black font-weight-bold'],
Copy link
Member

Choose a reason for hiding this comment

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

This won't do anything until cylc/cylc-flow#5713 is addressed, but this is how it should be

Suggested change
['this is a debug message', ''],
['this is a info message', 'bg-grey'],
['this is a warning message', 'bg-warning'],
['this is an error message', 'bg-error'],
['this is a critical message', 'bg-black font-weight-bold'],
['DEBUG', ''],
['INFO', 'bg-grey'],
['WARNING', 'bg-warning'],
['ERROR', 'bg-error'],
['CRITICAL', 'bg-black font-weight-bold'],

Can you take a look at

function jobMessageOutputs (jobNode) {

and add some logic to extract the severity level from the message which will be in the form SEVERITY:rest of message

Copy link
Member

Choose a reason for hiding this comment

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

(I've got a fix for the "output vs ordinary message" logic at #1456)

@@ -71,7 +71,8 @@ function jobMessageOutputs (jobNode) {
const ret = []
let messageOutput

for (const message of jobNode.node.messages || []) {
for (const messageString of jobNode.node.messages || []) {
const [level, message] = messageString.split(':')
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately as Oliver pointed out to me, we can't use String.split() as there could be multiple colons after the first severity level one. We'll have to use a regex here, I think something like

/^(?:(DEBUG|INFO|WARNING|ERROR|CRITICAL):)?(.*)/

@MetRonnie
Copy link
Member

Converted to draft pending resolution of cylc/cylc-flow#5714

@MetRonnie MetRonnie marked this pull request as draft October 5, 2023 14:31
@MetRonnie MetRonnie modified the milestones: 2.2.0, 2.x Oct 18, 2023
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.

color cylc message bubbles
3 participants