-
Notifications
You must be signed in to change notification settings - Fork 34
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
Update advisor.yml #260
Update advisor.yml #260
Conversation
Adding cards on the top from Advisor for Pathways, Incidents, Critical recommendations, Important recommendations.
@mmenestr does this make sense pls? |
data/advisor.yml
Outdated
@@ -3,6 +3,18 @@ advisor: | |||
color: 2 | |||
features: | |||
_scope: '[data-ouia-app-id="advisor"]' | |||
"Advisor - Card: Pathways": | |||
selectors: | |||
- '{}[data-testid="Pathways"]' |
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 think you need a space between the {} and the rest of the selector. Otherwise looks good.
Same for all of them
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.
but the rest of the existing ones do not have that space. Or am I misunderstanding the position of the space
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.
They should all have the space
https://github.com/RedHatInsights/pendo-config-automation/blob/main/docs/data.md#selectors
i made some spaces, but not sure if those are the ones mentioned in the comment
I added the spaces, hopefully i got it from reading the document and comments
Hey, sorry -- On my review, I was just talking about the ones that you added, not the rest of the file. Everything that was there already is spaced correctly. It's just due to how the selectors appear in the DOM, the ones you added need that extra space. |
i hope this is the one
we are getting there @ryelo hopefully this is the last edit 🔪 |
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.
Looks good now!
Adding cards on the top from Advisor for