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

[RHINENG-663] - Add subtitle property in AllServices #2576

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

AsToNlele
Copy link
Contributor

RHINENG-663

Is it possible to use the product field for the subtitle here?
image
At first I tried changing the bundle names, but that would not work for some ansible items

RedHatInsights/chrome-service-backend#228 depends on this PR

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2023

Codecov Report

Merging #2576 (0f428ba) into master (1e110a8) will not change coverage.
The diff coverage is n/a.

❗ Current head 0f428ba differs from pull request most recent head 8402333. Consider uploading reports for the commit 8402333 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #2576    +/-   ##
========================================
  Coverage   57.72%   57.72%            
========================================
  Files          92       92            
  Lines        2775     2775            
  Branches      707      653    -54     
========================================
  Hits         1602     1602            
- Misses       1059     1172   +113     
+ Partials      114        1   -113     
Impacted Files Coverage Δ
src/components/AllServices/allServicesLinks.ts 25.00% <ø> (ø)

... and 31 files with indirect coverage changes

Copy link
Contributor

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

Is it possible to use the product field for the subtitle here?

@AsToNlele unfortunately we can't as it would impact other tiles that do want to use the product as a subtitle and have the product defined in their links. I would suggest creating a new attribute inside the Chrome service as that instead.

@AsToNlele AsToNlele force-pushed the RHINENG-663 branch 2 times, most recently from e44fd49 to 7f06c74 Compare July 20, 2023 09:42
@AsToNlele AsToNlele changed the title [RHINENG-663] - Use Product as subtitle in AllServices [RHINENG-663] - Add subtitle property in AllServices Jul 20, 2023
@AsToNlele
Copy link
Contributor Author

I've added subtitle property


do not show bundle if the card title matches bundle title
*/}
<Text component="small">{subtitle ? subtitle : bundle !== title ? bundle : null}</Text>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change this to not use multiple ternary statements? To something like

subtitle || (bundle !== title ? bundle : null)

@AsToNlele AsToNlele marked this pull request as draft July 20, 2023 10:38
@AsToNlele AsToNlele marked this pull request as ready for review July 20, 2023 11:23
@Hyperkid123 Hyperkid123 merged commit 6e85e9c into RedHatInsights:master Jul 20, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants