-
Notifications
You must be signed in to change notification settings - Fork 25
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
Kuwait Theme: Component styling Updates #2521
base: master
Are you sure you want to change the base?
Changes from 28 commits
e2ceccb
ded4ce8
2d521bd
bea342b
f063957
ce170df
213f596
0d52291
1ed927b
059384c
1f278cd
24f4324
6b42bad
0fc821c
c7478e1
92e4ad3
fd56178
d5d695d
0cddf4f
136e6f9
9de2387
129c0fd
71bd2d3
7a08edd
950d93b
3f903f2
d155373
ce92cf4
16395e0
9a8c2ca
0c354fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
#!/bin/sh | ||
command -v git-lfs >/dev/null 2>&1 || { echo >&2 "\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting .git/hooks/pre-push.\n"; exit 2; } | ||
command -v git-lfs >/dev/null 2>&1 || { echo >&2 "\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'pre-push' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks').\n"; exit 2; } | ||
git lfs pre-push "$@" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,10 @@ | |
// NOTE - when copying css border-image double-slash // misinterpreted so replace with / 0 / | ||
|
||
$imageSize: 64px; | ||
$bubble-background-color-3: var(--text-bubble-background-color-3, --ion-color-primary-200); | ||
$bubble-border-color-3: var(--text-bubble-border-color-3, --ion-color-primary-500); | ||
$bubble-background-color-4: var(--text-bubble-background-color-4, --ion-color-secondary-200); | ||
$bubble-border-color-4: var(--text-bubble-border-color-4, --ion-color-secondary-500); | ||
|
||
.container { | ||
position: relative; | ||
|
@@ -32,6 +36,16 @@ $imageSize: 64px; | |
&[data-variant~="no_border"] { | ||
--border-color: transparent; | ||
} | ||
|
||
&[data-variant~="speaker-3"] { | ||
--background-color: #{$bubble-background-color-3}; | ||
--border-color: #{$bubble-border-color-3}; | ||
} | ||
|
||
&[data-variant~="speaker-4"] { | ||
--background-color: #{$bubble-background-color-4}; | ||
--border-color: #{$bubble-background-color-4}; | ||
} | ||
} | ||
|
||
.speaker-image { | ||
|
@@ -119,3 +133,21 @@ $imageSize: 64px; | |
max(var(--b), 100% - var(--p) - var(--h) * tan(var(--a) / 2)) 0 | ||
max(var(--b), var(--p) - var(--h) * tan(var(--a) / 2)) / 0 0 var(--h) 0; | ||
} | ||
|
||
// For the speaker name | ||
.speaker-name { | ||
position: absolute; | ||
bottom: -55px; | ||
left: 78px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit(non-blocking): I always start to get a little anxious when it comes to absolute-positioned elements. They tend to break quite quickly when making changes elsewhere, and often are harder to identify in the dom (appear out of order). Assuming this is the text that goes alongside an avatar/image then it might be worth trying to just wrap both in a flex box. If there is a strong preference to keep absolute positioning, I'd suggest using numbers which are a multiple of 16 or 8, as more likely than not these will align more closely with standard font sizes (16px). E.g. I'd set bottom as |
||
color: var(--ion-color-gray-700); | ||
} | ||
.container[data-position="right"] { | ||
.speaker-name { | ||
text-align: right; | ||
right: 80px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit(non-blocking): Just to quickly check, will this behave as expected when using a |
||
} | ||
img.speaker-image { | ||
-webkit-transform: scaleX(-1); | ||
transform: scaleX(-1); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,9 @@ interface ITextBubbleParams { | |
/** TEMPLATE PARAMETER: "speaker_position". The position of the speaker image and speech bubble tail */ | ||
speakerPosition: "left" | "right"; | ||
/** TEMPLATE PARAMETER: "variant" */ | ||
variant: "gray" | "primary" | "secondary" | "no-border"; | ||
variant: "gray" | "primary" | "secondary" | "no-border" | "speaker-3" | "speaker-4"; | ||
/** TEMPLATE PARAMETER: "avatar_name". The name of the speaker */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit(non-blocking): I assume this comment can also be updated now to Also not sure if we're really using a |
||
speakerName: string; | ||
} | ||
|
||
@Component({ | ||
|
@@ -36,5 +38,6 @@ export class TmplTextBubbleComponent extends TemplateBaseComponent implements On | |
this.params.variant = getStringParamFromTemplateRow(this._row, "variant", "") | ||
.split(",") | ||
.join(" ") as ITextBubbleParams["variant"]; | ||
this.params.speakerName = getStringParamFromTemplateRow(this._row, "speaker_name", ""); | ||
} | ||
} |
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.
In order to preserve consistency with non-Kuwait themes, we might need to make this element hidden on every other theme and only display it when the
plh_kids_kw
is active. Otherwise, we should style it so that it looks good on the other themes too. It would also be good to add an example to the comp_text_bubble template so that we can see how it displays