-
Notifications
You must be signed in to change notification settings - Fork 59
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: Adapt time to fiori3 #801
Conversation
Deploy preview for fundamental-styles ready! Built with commit 3541f5c |
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.
@@ -7,31 +7,221 @@ $block: #{$fd-namespace}-time; | |||
.#{$block} { | |||
@include fd-reset(); | |||
|
|||
width: 184px; // Quick fix to solve overflow issue | |||
$fd-time-item-height: 2.875rem; |
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 be 2 rem for cozy mode, it only gets bigger for smaller applications like tablet and phone
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.
Hello @stefan-sto compact mode has 2rem, default one has 2.875rem
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.
@JKMarkowski I think this is the wrong Stefano ;)
src/time.scss
Outdated
height: $fd-time-item-compact-height; | ||
overflow: hidden; | ||
white-space: nowrap; | ||
text-overflow: ellipsis; |
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.
we need to define a max-width for the overflow, ask the designers
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.
Hello @stefan-sto I think max width should be 100%
there. With text elipsis
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.
@JKMarkowski wrong Stefano
@trilodge can you review the PR from a11y point of view? |
we can review but merge once version |
src/time.scss
Outdated
display: flex; | ||
flex-direction: column; | ||
align-items: center; |
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.
You can use fd-flex-vertical-center() {flex-direction: column;}
src/time.scss
Outdated
display: flex; | ||
flex-direction: column; | ||
align-items: center; | ||
justify-content: center; |
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.
You can use @include fd-flex-center() {flex-direction: column;}
src/time.scss
Outdated
display: flex; | ||
align-items: center; | ||
justify-content: center; |
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.
@include fd-flex-center()
src/time.scss
Outdated
align-items: center; | ||
justify-content: center; |
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.
Is this working with display: block
?
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.
Leftovers :D
src/time.scss
Outdated
overflow: hidden; | ||
white-space: nowrap; | ||
text-overflow: ellipsis; |
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.
You can use @include fd-ellipsis()
@InnaAtanasova I just removed |
@stefanoScalzo Everything works fine, it's just about the uglify variables for tests |
Related Issue
Closes #577
Description
There is whole time component refactored, right now it follows all of fiori3 specs. Also there are SAP theming parameters used.
Breaking Changes:
Refactored markup for time/time picker.
Removed Classes:
fd-time__control
New modes:
fd-time(--compact/--tablet/--collapsible)
New elements:
fd-time__col
fd-time__slider-label
fd-time__wrapper(--active/--meridian)
fd-time__item(--collapsed)
fd-time__unit
Screenshots
Before:
TimePicker:
After:
Cozy:
Tablet:
Compact:
Time Picker: