-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Mobile UI optimization #5403
base: main
Are you sure you want to change the base?
Mobile UI optimization #5403
Conversation
@svrnm , please kindly review |
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.
assets/scss/_registry.scss
Outdated
@@ -9,7 +9,7 @@ | |||
@each $component, $color in $otel-registry-license-colors { | |||
&.badge-#{$component} { | |||
color: white; | |||
background-color: $color; | |||
background-color: $color; |
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.
background-color: $color; | |
background-color: $color; |
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.
@svrnm do i add a space here too?
@svrnm , i've applied a fix to this issue, kindly find attached a screenshot
|
</li> | ||
{{ end -}} | ||
{{ end -}} |
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.
{{ end -}} | |
{{ end -}} | |
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.
@svrnm please what fix exactly do you need here? the linting?
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.
Add an empty line at the bottom of the file
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.
Note that you might be able to auto fix that by running npm run:format
locally
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.
oh! @svrnm , thanks for this. I'm on it.
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.
@svrnm fixed, please review
Minor issue, but beyond that this looks good! When you have applied that minor change, I consider this as done! After the outreachy application phase we will take another look and see if we can merge this PR. |
</li> | ||
{{ end -}} | ||
{{ end -}} |
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.
{{ end -}} | |
{{ end -}} |
@kemsguy7 following up on this. Are you interested in continuing with this? If so, I would like to focus on "Changed display of filters and search input fields to flex column display for better visual alignment" and keep the other optimization out for now. Can you update this PR or create a new PR to address that? Thanks |
This pr resolves issue: #5318 where i resolved some ux issues on the mobile view of the registry page.
Key changes
Demo
mobile-optimization-open-telm.mp4