Skip to content
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

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
e2ceccb
refactor: text-bubble component
FaithDaka Nov 11, 2024
ded4ce8
styling: text-bubble
FaithDaka Nov 11, 2024
2d521bd
mid font variable
FaithDaka Nov 11, 2024
bea342b
refactor: speaker name
FaithDaka Nov 11, 2024
f063957
styling: text component
FaithDaka Nov 11, 2024
ce170df
new global variables
FaithDaka Nov 11, 2024
213f596
styling: text area component
FaithDaka Nov 11, 2024
0d52291
styling: text box component
FaithDaka Nov 11, 2024
1ed927b
fix: styling of audio and accordion components
FaithDaka Nov 12, 2024
059384c
Merge branch 'master' of https://github.com/IDEMSInternational/open-a…
FaithDaka Nov 12, 2024
1f278cd
Update comment
FaithDaka Nov 12, 2024
24f4324
fix: update class name
FaithDaka Nov 12, 2024
6b42bad
styling support for new variants and speaker name
FaithDaka Nov 12, 2024
0fc821c
refactor: variant names
FaithDaka Nov 12, 2024
c7478e1
additional color variants
FaithDaka Nov 12, 2024
92e4ad3
fix: use color variables
FaithDaka Nov 12, 2024
fd56178
Merge branch 'style/update-comp-styles' of https://github.com/IDEMSIn…
FaithDaka Nov 12, 2024
d5d695d
Merge branch 'master' of https://github.com/IDEMSInternational/open-a…
FaithDaka Nov 18, 2024
0cddf4f
chore: expose 'speaker_numer' parameter; expose tertiary and quartern…
jfmcquade Nov 18, 2024
136e6f9
chore: expose 'speaker_numer' parameter; expose tertiary and quartern…
jfmcquade Nov 18, 2024
9de2387
Merge branch 'style/update-comp-styles' of https://github.com/IDEMSIn…
jfmcquade Nov 18, 2024
129c0fd
Revert "chore: expose 'speaker_numer' parameter; expose tertiary and …
jfmcquade Nov 18, 2024
71bd2d3
chore: rename tertiary and quarternary text bubble variants, expose c…
jfmcquade Nov 18, 2024
7a08edd
fix: flip text bubble image at component level
FaithDaka Nov 19, 2024
950d93b
Merge branch 'style/update-comp-styles' of https://github.com/IDEMSIn…
FaithDaka Nov 19, 2024
3f903f2
fix: use colour variables
FaithDaka Nov 19, 2024
d155373
Merge branch 'master' of https://github.com/IDEMSInternational/open-a…
FaithDaka Nov 19, 2024
ce92cf4
Merge branch 'master' into style/update-comp-styles
esmeetewinkel Nov 21, 2024
16395e0
fix comment
FaithDaka Nov 25, 2024
9a8c2ca
Merge branch 'style/update-comp-styles' of https://github.com/IDEMSIn…
FaithDaka Nov 26, 2024
0c354fb
Merge branch 'master' of https://github.com/IDEMSInternational/open-a…
FaithDaka Nov 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .husky/pre-push
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
@@ -1,21 +1,28 @@
<div
class="container"
class="text-bubble-container container"
[attr.data-position]="params.speakerPosition"
[attr.data-variant]="params.variant"
>
<div class="text-bubble" [attr.data-position]="params.speakerPosition">
@if (_row.value) {
<plh-tmpl-text
[row]="{ _nested_name: '', name: '', type: 'text', value: value() }"
></plh-tmpl-text>
}
@for (childRow of _row.rows | filterDisplayComponent; track trackByRow($index, childRow)) {
<plh-template-component
[row]="childRow"
[parent]="parent"
[attr.data-rowname]="_row.name"
></plh-template-component>
<div>
@if (params.speakerName) {
Copy link
Collaborator

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

<div class="speaker-name">{{ params.speakerName }}</div>
} @else {
<div class="no-name"></div>
}
<div class="text-bubble" [attr.data-position]="params.speakerPosition">
@if (_row.value) {
<plh-tmpl-text
[row]="{ _nested_name: '', name: '', type: 'text', value: value() }"
></plh-tmpl-text>
}
@for (childRow of _row.rows | filterDisplayComponent; track trackByRow($index, childRow)) {
<plh-template-component
[row]="childRow"
[parent]="parent"
[attr.data-rowname]="_row.name"
></plh-template-component>
}
</div>
</div>
<img
[src]="params.speakerImageAsset | plhAsset"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The 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 -56px instead of -55, and left as 80px instead of 78 (unless wanting a more off-center appearance)

color: var(--ion-color-gray-700);
}
.container[data-position="right"] {
.speaker-name {
text-align: right;
right: 80px;
Copy link
Member

Choose a reason for hiding this comment

The 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 right-to-left language? Not sure in this case whether the text or avatar would be on the left/right (might be good to double-check)

}
img.speaker-image {
-webkit-transform: scaleX(-1);
transform: scaleX(-1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Copy link
Member

Choose a reason for hiding this comment

The 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 speaker_name.

Also not sure if we're really using a name or not here. Looking at the variants above I'm guessing we're more using a number to identify which speaker within a list of potential speakers that have different styling. If so might make more sense to use speaker_number. But I don't feel super strongly either way

speakerName: string;
}

@Component({
Expand Down Expand Up @@ -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", "");
}
}
45 changes: 45 additions & 0 deletions src/theme/themes/plh_kids_kw/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@

font-size-text-tiny: 14px,
font-size-text-small: 16px,
font-size-text-mid-size: 18px,
font-size-text-medium: 20px,
font-size-text-large: 24px,
font-size-text-title: 28px,
font-size-text-extra-large: 32px,

line-height-text-tiny: 18px,
line-height-text-small: 24px,
line-height-text-medium: 28px,
Expand Down Expand Up @@ -81,6 +84,48 @@
icon-size-large: 32px,
icon-size-extra-large: 40px,
icon-size-largest: 48px,

paragraph-margin-small: 4px,
paragraph-margin-medium: 8px,
paragraph-margin-large: 12px,

// SURFACE COLOUR PALETTE
color-surface-white: #ffffff,
color-surface-white-variant: #f9f9fa,
color-surface-black: #1b1c1d,
color-surface-black-variant: #44474a,
color-outline: #74777c,
color-outline-variant: #c4c7c9,

// SECONDARY PALETTE
color-secondary-blue-40: #0066a1,
color-secondary-blue-50: #007fbf,
color-secondary-blue-60: #2e98d8,
color-secondary-blue-70: #6ab2e9,
color-secondary-blue-80: #9ecbf5,
color-secondary-blue-90: #cfe5fb,

// YELLOW PALETTE
color-accent-yellow-40: #815600,
color-accent-yellow-50: #9f6e00,
color-accent-yellow-60: #bb8807,
color-accent-yellow-70: #d5a344,
color-accent-yellow-80: #eac07c,
color-accent-yellow-95: #fdefdc,

// ORANGE PALETTE
color-accent-orange-40: #b82000,
color-accent-orange-50: #dd3b00,
color-accent-orange-70: #ff8250,
color-accent-orange-80: #ffab83,
color-accent-orange-90: #ffd5be,
color-accent-orange-95: #ffeade,

// TEXT BUBBLE COLOURS
text-bubble-background-color-3: var(--color-accent-orange-95),
text-bubble-border-color-3: var(--color-accent-orange-70),
text-bubble-background-color-4: var(--color-accent-yellow-95),
text-bubble-border-color-4: var(--color-accent-yellow-70),
);
@include utils.generateTheme($color-primary, $color-secondary, $page-background);
@each $name, $value in $variable-overrides {
Expand Down
Loading
Loading