-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Revamp widget parameterization #14326
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
base: main
Are you sure you want to change the base?
Conversation
Add a status badge to display the count of translated languages for a project or component.
Revamp the widgets page. Use a card based layout for settings, preview and showing the embed code. The current refactoring unifies the settings UI on the frontend and generates the urls for various widgets on the frontend as well. Currently, this serves no extra benefit but will prove to be helpful in future when we add additional settings which are only applicable to specific widgets.
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.
There are accessibility issues in these changes.
weblate/static/js/widgets.js
Outdated
widget.extra_parameters.forEach(param => { | ||
let input; | ||
if (param.type === 'number') { | ||
input = $('<input/>', { |
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.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
|
||
<div class="widget-card bg-white p-3"> | ||
<h4>Embed Code</h4> | ||
<textarea id="embedCode" class="code-example form-control" rows="3" readonly> |
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.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
for more information, see https://pre-commit.ci
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.
There are accessibility issues in these changes.
widget.extra_parameters.forEach((param) => { | ||
let input; | ||
if (param.type === "number") { | ||
input = $("<input/>", { |
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.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
|
||
<div class="widget-card bg-white p-3"> | ||
<h4>Embed Code</h4> | ||
<textarea id="embedCode" class="code-example form-control" rows="3" readonly> |
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.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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 think this goes into the correct direction. I'm not sure about the frontend code, I will add @meel-hd to review that.
"social:begin", | ||
"djangosaml2idp:saml_login_process", | ||
} | ||
INLINE_PATHS = {"social:begin", "djangosaml2idp:saml_login_process", "widgets"} |
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.
Please avoid this, these are third-party apps where we can't avoid that, but our code should not use inline javascript.
@@ -1,4 +1,4 @@ | |||
# Copyright © Michal Čihař <michal@weblate.org> | |||
# Copyright Michal Čihař <michal@weblate.org> |
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.
Seems like unintended change.
</div> | ||
|
||
{% endblock %} | ||
|
||
{% block extra_script %} | ||
<script type="application/json" id="widgets-data">{{ widgets_json|safe }}</script> |
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 don't think safe should be used here. The JSON can still contain markup and that would break. Also passing this as data attribute would be better approach as that would not need inline scripts in CSP and you would have no issues with escaping.
border: 1px solid var(--altcha-color-border); | ||
border-radius: var(--altcha-border-radius); |
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.
These variables are specific to a 3rd party lib, use the more general alternatives defined in variables.css
box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1); | ||
margin-bottom: 20px; | ||
padding: 15px; | ||
background-color: #fff; |
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.
Update this in dark-styles.css
to be suitable for dark theme
Based on #13051 (comment), this PR explores a new way to parameterize widgets. Here is the what the proposed UI looks like:
The widget, language, component, code, and color options are available for all widgets. All the extra options per widget are defined in the
extra_parameters
attribute of the widget class and passed through JSON where jQuery reads it and renders it. Using the options, jQuery generates the urls dynamically, and updates the live preview and the code to embed the images.Also, this is just a draft to start discussion.