-
Notifications
You must be signed in to change notification settings - Fork 985
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
Improved accessibility for static labels and chips #8239
Improved accessibility for static labels and chips #8239
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8239 +/- ##
=======================================
Coverage 66.29% 66.30%
=======================================
Files 597 597
Lines 60949 60949
Branches 6258 6262 +4
=======================================
+ Hits 40407 40411 +4
+ Misses 20542 20538 -4 ☔ View full report in Codecov by Sentry. |
f27aa82
to
d8553e5
Compare
4d383d7
to
9fb1918
Compare
77c49f4
to
4087e2d
Compare
f8359b7
to
558e6c1
Compare
f137c15
to
50b38e5
Compare
opencti-platform/opencti-front/src/components/ItemAccountStatus.tsx
Outdated
Show resolved
Hide resolved
opencti-platform/opencti-front/src/components/ItemConfidence.tsx
Outdated
Show resolved
Hide resolved
...tform/opencti-front/src/private/components/common/stix_core_objects/StixCoreObjectLabels.tsx
Outdated
Show resolved
Hide resolved
...common/stix_core_objects_or_stix_relationships/StixCoreObjectOrCoreRelationshipLabelsView.js
Outdated
Show resolved
Hide resolved
...platform/opencti-front/src/private/components/entities/organizations/OrganizationDetails.tsx
Outdated
Show resolved
Hide resolved
...platform/opencti-front/src/private/components/entities/organizations/OrganizationDetails.tsx
Outdated
Show resolved
Hide resolved
...platform/opencti-front/src/private/components/entities/organizations/OrganizationDetails.tsx
Outdated
Show resolved
Hide resolved
...platform/opencti-front/src/private/components/entities/organizations/OrganizationDetails.tsx
Outdated
Show resolved
Hide resolved
opencti-platform/opencti-front/src/components/ItemConfidence.tsx
Outdated
Show resolved
Hide resolved
opencti-platform/opencti-front/src/components/ItemLikelihood.jsx
Outdated
Show resolved
Hide resolved
50b38e5
to
151dfdd
Compare
style={{ | ||
...classStyle, | ||
color: theme.palette.chip.main, | ||
borderColor: monochrome_labels ? theme.palette.background.accent : classStyle.borderColor, |
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.
Just having a thought right now : wouldn't it be better to have a attribute in the chip if we want the monochrome or the normal behavior and have the theme.palette.... inside all chips directly ?
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 like we're using the MUI chip, so we could create a wrapper for it to determine monochromatic label status and apply the appropriate colors/styling.
Ideally, at least in my opinion, we'd extend the Theme interface and then be able to create a custom theme like light mode or dark mode then be able to switch to a monochromatic theme instead of all these ternary checks. At the moment, I think the platform is currently set to only handle dark or light mode themes.
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.
That actually is a good idea, why don't we chose that path ?
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 talked to @Jipegien about it over email/conversation and he said to wait for feedback before implementing this expanded theme approach.
aea83f3
to
ea12749
Compare
ea12749
to
7789251
Compare
7789251
to
e8e0d17
Compare
e8e0d17
to
80669ec
Compare
Opening new PR for customizable themes concept instead of this one. |
Proposed changes
Related issues
Checklist
Further comments
I didn't write auto tests since they are mostly style changes