Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(ui5-rating-indicator): initial implementation #1729
feat(ui5-rating-indicator): initial implementation #1729
Changes from 4 commits
8b00c41
3426abe
180611e
c271d0a
5e3e0c4
15de6b9
11682a2
37a00b2
b3c2023
04d7836
b60c8ba
818e1d8
8e5b39f
48c27b1
869ea78
38779f5
4eb6d52
aa417bf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 am not sure about the name of the event. Indeed for Input, Combo, etc we decided to use "input" instead of "liveChange", but for the RatingIndicator, the user does not type anything. Maybe here live-change is a little better.
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.
Should we fire value-changed for Angular here and in the rest of the places you fire input,change?
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.
With Space and Enter, after the values reaches the maxValue (all starts are active) it starts over, it is cycling behaviour basically. This is how in openui5 works at least.
With Up/Left and Down/Right it is like currently works.
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.
The focus should be just around the stars:
currently is:
The problem is not with the css for the outline, but the stars itself or the wrapper.