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.

Sign up for GitHub

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: social sources redesign #750

Merged
merged 12 commits into from
Dec 4, 2024
Merged

feat: social sources redesign #750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fdfda4c
feat: new social sources
nd0ut Oct 15, 2024
f856136
chore: extract types to separate file
nd0ut Oct 15, 2024
dcf0868
chore: reduce theme variables list
nd0ut Oct 15, 2024
2af67db
chore: reset selection state on unmount
nd0ut Oct 15, 2024
79fc9fc
chore: add social sourcesdemo page
nd0ut Oct 23, 2024
ed07398
chore: fix activity width
nd0ut Oct 24, 2024
1c96ee3
chore: disable scale for social source demo
nd0ut Oct 25, 2024
d8e6555
chore: increase modal size & refactor modal css & remove dialog api f…
nd0ut Oct 25, 2024
3d45c44
chore: fix upload list height on safari
nd0ut Oct 25, 2024
6a1449a
chore: added l10n
egordidenko Nov 20, 2024
9fdd697
chore: cleanup
nd0ut Dec 4, 2024
bd9b7cf
chore(l10n): add missing keys for all the locales
nd0ut Dec 4, 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: 0 additions & 2 deletions blocks/CameraSource/CameraSource.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ export class CameraSource extends UploaderBlock {
* @param {'granted' | 'denied' | 'prompt'} state
*/
_setPermissionsState = debounce((state) => {
this.classList.toggle('uc-initialized', state === 'granted');

if (state === 'granted') {
this.set$({
videoHidden: false,
Expand Down
19 changes: 4 additions & 15 deletions blocks/CameraSource/camera-source.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,9 @@ uc-camera-source {
border-radius: var(--uc-radius);
}

[uc-modal] uc-camera-source {
width: min(calc(var(--uc-dialog-max-width) - var(--uc-padding) * 2), calc(100vw - var(--uc-padding) * 2));
height: 100vh;
max-height: var(--modal-max-content-height);
}

uc-camera-source.uc-initialized {
height: max-content;
}

@media only screen and (max-width: 430px) {
uc-camera-source {
width: calc(100vw - var(--uc-padding) * 2);
height: var(--modal-content-height-fill, 100%);
}
[uc-modal] > dialog:has(uc-camera-source[active]) {
width: 100%;
height: 100%;
}

uc-camera-source video {
Expand Down Expand Up @@ -52,6 +40,7 @@ uc-camera-source .uc-content {
flex: 1;
justify-content: center;
width: 100%;
height: 100%;
padding: var(--uc-padding);
padding-top: 0;
overflow: hidden;
Expand Down
6 changes: 3 additions & 3 deletions blocks/CloudImageEditorActivity/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ uc-cloud-image-editor-activity {
background-color: var(--uc-background);
}

[uc-modal] uc-cloud-image-editor-activity {
width: min(calc(var(--uc-dialog-max-width) - var(--uc-padding) * 2), calc(100vw - var(--uc-padding) * 2));
height: var(--modal-content-height-fill, 100%);
[uc-modal] > dialog:has(uc-cloud-image-editor-activity[active]) {
width: 100%;
height: 100%;
Comment on lines +10 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

:has() selector usage needs fallback styles

The codebase search reveals that while there are default dialog styles in blocks/Modal/modal.css using :where([uc-modal]) > dialog, these don't fully address the width/height requirements when specific components are active. The :has() selector is used to apply full-width/height styles for certain components, but there's no fallback mechanism for browsers that don't support it.

  • blocks/Modal/modal.css provides base dialog styles with fixed widths
  • Components like cloud-image-editor, camera-source, and external-source require full-width/height when active
  • No alternative styling mechanism exists for browsers without :has() support

Recommendation: Add fallback styles using a parent class or attribute that can be toggled via JavaScript when these components are active. For example:

[uc-modal][full-size] > dialog,
[uc-modal] > dialog:has(uc-cloud-image-editor-activity[active]) {
  width: 100%;
  height: 100%;
}
🔗 Analysis chain

Potential browser compatibility issues with the :has() pseudo-class

The :has() CSS pseudo-class is not fully supported in all browsers, which may lead to layout issues for some users. Ensure that the target browsers support :has(), or provide a fallback for those that do not.

Run the following script to find all usages of :has() and verify browser compatibility:

:has() selector usage requires fallback styles for older browsers

Based on the search results, the :has() selector is used consistently across multiple CSS files in modal dialogs and file uploader components. This pattern affects core UI functionality and needs fallback styles to ensure compatibility with older browsers. Here are the affected files:

  • blocks/CloudImageEditorActivity/index.css:10
  • blocks/StartFrom/start-from.css:16
  • blocks/ExternalSource/external-source.css:11
  • blocks/CameraSource/camera-source.css:13
  • solutions/file-uploader/inline/index.css (multiple instances)

Let's verify if there are any existing fallback styles:

Let me try one more search to check for any default dialog styles or JavaScript-based fallbacks:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for all CSS files using ':has()' and list them.

# Find all CSS files containing ':has('
fd -e css -x grep -Hn ':has(' {}

# Check browser compatibility for ':has()' pseudo-class.
echo "Refer to https://caniuse.com/css-has for browser support details."

Length of output: 989

Script:

#!/bin/bash
# Search for alternative styling approaches for these components
# that might serve as fallbacks

# Check for dialog styling without :has()
rg -g "*.css" "dialog\s*{|dialog\[|dialog\." -A 2

# Check for component-specific styling without :has()
rg -g "*.css" "uc-cloud-image-editor-activity\[active\]|uc-start-from\[active\]|uc-external-source\[active\]|uc-camera-source\[active\]" -A 2

Length of output: 1136

Script:

#!/bin/bash
# Search for default dialog styles and potential JS-based solutions
rg -g "*.css" "dialog[^:]" -A 2

# Search for dialog-related JavaScript that might handle fallbacks
rg -g "*.{js,ts}" "dialog.*active" -A 2

Length of output: 4628

}
195 changes: 96 additions & 99 deletions blocks/ExternalSource/ExternalSource.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,12 @@
import { UploaderBlock } from '../../abstract/UploaderBlock.js';
import { stringToArray } from '../../utils/stringToArray.js';
import { wildcardRegexp } from '../../utils/wildcardRegexp.js';
import { buildStyles } from './buildStyles.js';
import { registerMessage, unregisterMessage } from './messages.js';
import { buildThemeDefinition } from './buildThemeDefinition.js';
import { MessageBridge } from './MessageBridge.js';
import { queryString } from './query-string.js';

/** @typedef {{ externalSourceType: string }} ActivityParams */

/**
* @typedef {{
* type: 'file-selected';
* obj_type: 'selected_file';
* filename: string;
* url: string;
* alternatives?: Record<string, string>;
* }} SelectedFileMessage
*/

/**
* @typedef {{
* type: 'embed-css';
* style: string;
* }} EmbedCssMessage
*/

/** @typedef {SelectedFileMessage | EmbedCssMessage} Message */

export class ExternalSource extends UploaderBlock {
couldBeCtxOwner = true;
activityType = ActivityBlock.activities.EXTERNAL;
Expand All @@ -41,12 +22,20 @@
...this.init$,
activityIcon: '',
activityCaption: '',

/** @type {import('./types.js').InputMessageMap['selected-files-change']['selectedFiles']} */
selectedList: [],
counter: 0,
multiple: false,
total: 0,

isSelectionReady: false,
couldSelectAll: false,
couldDeselectAll: false,
showSelectionStatus: false,
counterText: '',

onDone: () => {
for (const message of this.$.selectedList) {
const url = this.extractUrlFromMessage(message);
const url = this.extractUrlFromSelectedFile(message);
const { filename } = message;
const { externalSourceType } = this.activityParams;
this.api.addFileFromUrl(url, { fileName: filename, source: externalSourceType });
Expand All @@ -57,6 +46,14 @@
onCancel: () => {
this.historyBack();
},

onSelectAll: () => {
this._messageBridge?.send({ type: 'select-all' });
},

onDeselectAll: () => {
this._messageBridge?.send({ type: 'deselect-all' });
},
};
}

Expand All @@ -69,12 +66,6 @@
throw new Error(`External Source activity params not found`);
}

/**
* @private
* @type {HTMLIFrameElement | null}
*/
_iframe = null;

initCallback() {
super.initCallback();
this.registerActivity(this.activityType, {
Expand All @@ -96,7 +87,7 @@
this.mountIframe();
},
});
this.sub('*currentActivityParams', (val) => {

Check warning on line 90 in blocks/ExternalSource/ExternalSource.js

View workflow job for this annotation

GitHub Actions / build 

'val' is defined but never used
if (!this.isActivityActive) {
return;
}
Expand All @@ -108,106 +99,97 @@
this.unmountIframe();
}
});
this.sub('selectedList', (list) => {
this.$.counter = list.length;
});
this.subConfigValue('multiple', (multiple) => {
this.$.multiple = multiple;
this.$.showSelectionStatus = multiple;
});

this.subConfigValue('localeName', (val) => {

Check warning on line 106 in blocks/ExternalSource/ExternalSource.js

View workflow job for this annotation

GitHub Actions / build 

'val' is defined but never used
this.setupL10n();
});
}

/**
* @private
* @param {SelectedFileMessage} message
* @param {NonNullable<import('./types.js').InputMessageMap['selected-files-change']['selectedFiles']>[number]} selectedFile
*/
extractUrlFromMessage(message) {
if (message.alternatives) {
extractUrlFromSelectedFile(selectedFile) {
if (selectedFile.alternatives) {
const preferredTypes = stringToArray(this.cfg.externalSourcesPreferredTypes);
for (const preferredType of preferredTypes) {
const regexp = wildcardRegexp(preferredType);
for (const [type, typeUrl] of Object.entries(message.alternatives)) {
for (const [type, typeUrl] of Object.entries(selectedFile.alternatives)) {
if (regexp.test(type)) {
return typeUrl;
}
}
}
}

return message.url;
return selectedFile.url;
}

/**
* @private
* @param {Message} message
* @param {import('./types.js').InputMessageMap['selected-files-change']} message
*/
sendMessage(message) {
this._iframe?.contentWindow?.postMessage(JSON.stringify(message), '*');
}

/**
* @private
* @param {SelectedFileMessage} message
*/
async handleFileSelected(message) {
if (!this.$.multiple && this.$.selectedList.length) {
async handleSelectedFilesChange(message) {
if (this.cfg.multiple !== message.isMultipleMode) {
console.error('Multiple mode mismatch');
return;
}

this.$.selectedList = [...this.$.selectedList, message];

if (!this.$.multiple) {
this.$.onDone();
}
this.bindL10n('counterText', () =>
this.l10n('selected-count', {
count: message.selectedCount,
total: message.total,
}),
);
Comment on lines +141 to +146
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid rebinding localization in handleSelectedFilesChange

Calling this.bindL10n inside handleSelectedFilesChange may cause multiple bindings with each call. Initialize the binding once in initCallback and update the values separately.

Move the binding to initCallback:

initCallback() {
  super.initCallback();
+ this.bindL10n('counterText', () =>
+   this.l10n('selected-count', {
+     count: this.$.selectedCount,
+     total: this.$.total,
+   }),
+ );

  // Existing code...
}

Update handleSelectedFilesChange to set selectedCount and total:

this.set$({
  isSelectionReady: message.isReady,
  showSelectionStatus: message.isMultipleMode && message.total > 0,
  couldSelectAll: message.selectedCount < message.total,
  couldDeselectAll: message.selectedCount === message.total,
  selectedList: message.selectedFiles,
+ selectedCount: message.selectedCount,
+ total: message.total,
});

Committable suggestion skipped: line range outside the PR's diff.


this.set$({
isSelectionReady: message.isReady,
showSelectionStatus: message.isMultipleMode && message.total > 0,
couldSelectAll: message.selectedCount < message.total,
couldDeselectAll: message.selectedCount === message.total,
selectedList: message.selectedFiles,
});
}

/** @private */
handleIframeLoad() {
this.applyStyles();
}

/**
* @private
* @param {string} propName
*/
getCssValue(propName) {
let style = window.getComputedStyle(this);
return style.getPropertyValue(propName).trim();
this.setupL10n();
}

/** @private */
applyStyles() {
let colors = {
radius: this.getCssValue('--uc-radius'),
backgroundColor: this.getCssValue('--uc-background'),
textColor: this.getCssValue('--uc-foreground'),
secondaryColor: this.getCssValue('--uc-secondary'),
secondaryForegroundColor: this.getCssValue('--uc-secondary-foreground'),
secondaryHover: this.getCssValue('--uc-secondary-hover'),
linkColor: this.getCssValue('--uc-primary'),
linkColorHover: this.getCssValue('--uc-primary-hover'),
fontFamily: this.getCssValue('--uc-font-family'),
fontSize: this.getCssValue('--uc-font-size'),
};
this._messageBridge?.send({
type: 'set-theme-definition',
theme: buildThemeDefinition(this),
});
}

this.sendMessage({
type: 'embed-css',
style: buildStyles(colors),
/** @private */
setupL10n() {
this._messageBridge?.send({
type: 'set-locale-definition',
localeDefinition: this.cfg.localeName,
});
}

/** @private */
remoteUrl() {
const { pubkey, remoteTabSessionKey, socialBaseUrl } = this.cfg;
const { pubkey, remoteTabSessionKey, socialBaseUrl, multiple } = this.cfg;
const { externalSourceType } = this.activityParams;
const lang = this.l10n('social-source-lang')?.split('-')?.[0] || 'en';
const params = {
lang,
public_key: pubkey,
images_only: false.toString(),
pass_window_open: false,
session_key: remoteTabSessionKey,
wait_for_theme: true,
multiple: multiple.toString(),
};
const url = new URL(`/window3/${externalSourceType}`, socialBaseUrl);
const url = new URL(`/window4/${externalSourceType}`, socialBaseUrl);
url.search = queryString(params);
return url.toString();
}
Expand All @@ -231,31 +213,43 @@
this.ref.iframeWrapper.innerHTML = '';
this.ref.iframeWrapper.appendChild(iframe);

registerMessage('file-selected', iframe.contentWindow, this.handleFileSelected.bind(this));
if (!iframe.contentWindow) {
return;
}

this._messageBridge?.destroy();

/** @private */
this._messageBridge = new MessageBridge(iframe.contentWindow);
this._messageBridge.on('selected-files-change', this.handleSelectedFilesChange.bind(this));

this._iframe = iframe;
this.$.selectedList = [];
this.resetSelectionStatus();
}

/** @private */
unmountIframe() {
this._iframe && unregisterMessage('file-selected', this._iframe.contentWindow);
this._messageBridge?.destroy();
this._messageBridge = undefined;
this.ref.iframeWrapper.innerHTML = '';
this._iframe = null;
this.$.selectedList = [];
this.$.counter = 0;

this.resetSelectionStatus();
}

/** @private */
resetSelectionStatus() {
this.set$({
selectedList: [],
total: 0,
isSelectionReady: false,
couldSelectAll: false,
couldDeselectAll: false,
showSelectionStatus: false,
});
}
}

ExternalSource.template = /* HTML */ `
<uc-activity-header>
<button type="button" class="uc-mini-btn" set="onclick: *historyBack" l10n="@title:back">
<uc-icon name="back"></uc-icon>
</button>
<div>
<uc-icon set="@name: activityIcon"></uc-icon>
<span>{{activityCaption}}</span>
</div>
<button
type="button"
class="uc-mini-btn uc-close-btn"
Expand All @@ -269,12 +263,15 @@
<div ref="iframeWrapper" class="uc-iframe-wrapper"></div>
<div class="uc-toolbar">
<button type="button" class="uc-cancel-btn uc-secondary-btn" set="onclick: onCancel" l10n="cancel"></button>
<div></div>
<div set="@hidden: !multiple" class="uc-selected-counter"><span l10n="selected-count"></span>{{counter}}</div>
<div set="@hidden: !showSelectionStatus" class="uc-selection-status-box">
<span>{{counterText}}</span>
<button type="button" set="onclick: onSelectAll; @hidden: !couldSelectAll" l10n="select-all"></button>
<button type="button" set="onclick: onDeselectAll; @hidden: !couldDeselectAll" l10n="deselect-all"></button>
</div>
<button
type="button"
class="uc-done-btn uc-primary-btn"
set="onclick: onDone; @disabled: !counter"
set="onclick: onDone; @disabled: !isSelectionReady"
l10n="done"
></button>
</div>
Expand Down
Loading
Loading