Skip to content

Conversation

@kugelblittzz
Copy link
Contributor

@kugelblittzz kugelblittzz commented May 13, 2023

This PR is related to the issue #7056.

backgroundColor of user-menu-avatar was hardcoded to primary color value instead of accessing it from @primary-color in config.json file.

Description

This PR resolves #7056.

backgroundColor of user-menu-avatar was hardcoded to primary color value instead of accessing @primary-color in config.json file.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

…lue instead of accessing it from @primary-color in config.json file. Updated the code to access the primary color from config.json
@boring-cyborg
Copy link

boring-cyborg bot commented May 13, 2023

Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:

@yadvr
Copy link
Member

yadvr commented May 18, 2023

@blueorangutan ui

@blueorangutan
Copy link

@rohityadavcloud a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link

UI build failed: ✖️
(SL-JID-107)

@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #7532 (a73fb23) into main (2f309b5) will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #7532   +/-   ##
=========================================
  Coverage     12.95%   12.95%           
- Complexity     8986     8987    +1     
=========================================
  Files          2728     2728           
  Lines        256647   256647           
  Branches      40024    40024           
=========================================
+ Hits          33256    33258    +2     
+ Misses       219214   219208    -6     
- Partials       4177     4181    +4     

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm

@kugelblittzz
Copy link
Contributor Author

Is there anything else that i have to do?

@DaanHoogland
Copy link
Contributor

Is there anything else that i have to do?

@ilhamalrahm , You need a second LGTM and test verification. Best is to look at git blame to see who else modified the code in that area and ask them for a review.
It also help to fill the How was this tested section, so it will be easy for someone else to replicate.

@kugelblittzz
Copy link
Contributor Author

@davidjumani and @utchoang worked last in that area. How can i request for a review?

@DaanHoogland
Copy link
Contributor

@davidjumani and @utchoang worked last in that area. How can i request for a review?

ok, these are not currently active on the project @shwstppr can you review?

@kugelblittzz
Copy link
Contributor Author

Anything i can do?

@shwstppr
Copy link
Contributor

@blueorangutan ui

@blueorangutan
Copy link

@shwstppr a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/7532 (QA-JID-120)

@soreana
Copy link
Member

soreana commented Jun 20, 2023

Tested manually, works as expected.

Before change:

Screenshot 2023-06-20 at 16 13 23

After change:

Screenshot 2023-06-20 at 16 13 35

@DaanHoogland DaanHoogland requested a review from soreana June 20, 2023 15:11
@DaanHoogland DaanHoogland merged commit 071a071 into apache:main Jun 21, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Jun 21, 2023

Awesome work, congrats on your first merged pull request!

@DaanHoogland DaanHoogland added this to the 4.19.0.0 milestone Sep 29, 2023
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.

UI user icon not using primary-color setting

6 participants