From 0e362b7678e75cb71ce207fd45dd4dc0d955fdca Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 28 Apr 2020 10:16:55 +0200 Subject: [PATCH] Fix end-user-facing uses of inline CSS (#13438) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Move some inline styles to CSS files * Move default_account_display_name span to fix useless tags with duplicate id * Change handling of public pages spoiler text from inline CSS to dataset attribute * Use the `dir` HTML attribute instead of inline CSS * Move status action bar inline CSS to CSS file * Hide logo resources from CSS file, not inline CSS Fixes #11601 * Move translation prompt styling from inline CSS to CSS file * Move “invited by” styling on registration form from inline to CSS file * Use the progress tag to display poll results in JS fallback * Fix poll results JS-less fallback when the user has voted for an option * Change account public page “moved” notice to use img tags instead of inline CSS * Move OTP hint inline CSS to SCSS file * Hide JS-less fallback vote progressbars from accessibility tools Co-authored-by: Eugen Rochko --- app/javascript/packs/public.js | 16 +++++----- app/javascript/styles/mastodon/about.scss | 5 ++++ app/javascript/styles/mastodon/basics.scss | 4 +++ .../styles/mastodon/components.scss | 12 ++++++++ app/javascript/styles/mastodon/forms.scss | 21 +++++++++++++ app/javascript/styles/mastodon/polls.scss | 30 +++++++++++++++++++ app/javascript/styles/mastodon/statuses.scss | 17 +++++++++++ app/views/about/show.html.haml | 6 ++-- app/views/accounts/_moved.html.haml | 6 ++-- app/views/application/_card.html.haml | 1 - app/views/auth/registrations/new.html.haml | 4 +-- app/views/auth/sessions/two_factor.html.haml | 2 +- app/views/directories/index.html.haml | 1 - app/views/layouts/application.html.haml | 2 +- app/views/layouts/embedded.html.haml | 2 +- .../preferences/appearance/show.html.haml | 4 +-- app/views/settings/profiles/show.html.haml | 2 +- app/views/statuses/_detailed_status.html.haml | 6 ++-- app/views/statuses/_poll.html.haml | 8 +++-- app/views/statuses/_simple_status.html.haml | 12 ++++---- 20 files changed, 125 insertions(+), 36 deletions(-) diff --git a/app/javascript/packs/public.js b/app/javascript/packs/public.js index 85789c8aa..557823c96 100644 --- a/app/javascript/packs/public.js +++ b/app/javascript/packs/public.js @@ -120,15 +120,13 @@ function main() { delegate(document, '.custom-emoji', 'mouseout', getEmojiAnimationHandler('data-static')); delegate(document, '.status__content__spoiler-link', 'click', function() { - const contentEl = this.parentNode.parentNode.querySelector('.e-content'); + const statusEl = this.parentNode.parentNode; - if (contentEl.style.display === 'block') { - contentEl.style.display = 'none'; - this.parentNode.style.marginBottom = 0; + if (statusEl.dataset.spoiler === 'expanded') { + statusEl.dataset.spoiler = 'folded'; this.textContent = (new IntlMessageFormat(messages['status.show_more'] || 'Show more', locale)).format(); } else { - contentEl.style.display = 'block'; - this.parentNode.style.marginBottom = null; + statusEl.dataset.spoiler = 'expanded'; this.textContent = (new IntlMessageFormat(messages['status.show_less'] || 'Show less', locale)).format(); } @@ -136,8 +134,8 @@ function main() { }); [].forEach.call(document.querySelectorAll('.status__content__spoiler-link'), (spoilerLink) => { - const contentEl = spoilerLink.parentNode.parentNode.querySelector('.e-content'); - const message = (contentEl.style.display === 'block') ? (messages['status.show_less'] || 'Show less') : (messages['status.show_more'] || 'Show more'); + const statusEl = spoilerLink.parentNode.parentNode; + const message = (statusEl.dataset.spoiler === 'expanded') ? (messages['status.show_less'] || 'Show less') : (messages['status.show_more'] || 'Show more'); spoilerLink.textContent = (new IntlMessageFormat(message, locale)).format(); }); }); @@ -170,7 +168,7 @@ function main() { if (target.value) { name.innerHTML = emojify(escapeTextContentForBrowser(target.value)); } else { - name.textContent = document.querySelector('#default_account_display_name').textContent; + name.textContent = target.dataset.default; } } }); diff --git a/app/javascript/styles/mastodon/about.scss b/app/javascript/styles/mastodon/about.scss index cf16b54ac..711f34965 100644 --- a/app/javascript/styles/mastodon/about.scss +++ b/app/javascript/styles/mastodon/about.scss @@ -757,8 +757,13 @@ $small-breakpoint: 960px; } } + &__counters__wrapper { + display: flex; + } + &__counter { padding: 10px; + width: 50%; strong { font-family: $font-display, sans-serif; diff --git a/app/javascript/styles/mastodon/basics.scss b/app/javascript/styles/mastodon/basics.scss index 4eff8a465..a5dbe75fb 100644 --- a/app/javascript/styles/mastodon/basics.scss +++ b/app/javascript/styles/mastodon/basics.scss @@ -230,6 +230,10 @@ button { } } +.logo-resources { + display: none; +} + // NoScript adds a __ns__pop2top class to the full ancestry of blocked elements, // to set the z-index to a high value, which messes with modals and dropdowns. // Blocked elements can in theory only be media and frames/embeds, so they diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index beb3b3cfd..e22b87711 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -1362,6 +1362,12 @@ a .account__avatar { &-base { @include avatar-radius; @include avatar-size(36px); + + img { + @include avatar-radius; + width: 100%; + height: 100%; + } } &-overlay { @@ -1372,6 +1378,12 @@ a .account__avatar { bottom: 0; right: 0; z-index: 1; + + img { + @include avatar-radius; + width: 100%; + height: 100%; + } } } diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index c9ad68f94..0e5b00e8f 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -142,6 +142,10 @@ code { } } + .otp-hint { + margin-bottom: 25px; + } + .card { margin-bottom: 15px; } @@ -285,6 +289,14 @@ code { margin-bottom: 25px; } } + + .fields-group.invited-by { + margin-bottom: 30px; + + .hint { + text-align: center; + } + } } .input.radio_buttons .radio label { @@ -635,6 +647,15 @@ code { @media screen and (max-width: 740px) and (min-width: 441px) { margin-top: 40px; } + + &.translation-prompt { + text-align: unset; + color: unset; + + a { + text-decoration: underline; + } + } } .form-footer { diff --git a/app/javascript/styles/mastodon/polls.scss b/app/javascript/styles/mastodon/polls.scss index 1ecc8434d..ad7088982 100644 --- a/app/javascript/styles/mastodon/polls.scss +++ b/app/javascript/styles/mastodon/polls.scss @@ -19,6 +19,36 @@ } } + progress { + border: 0; + display: block; + width: 100%; + height: 5px; + appearance: none; + background: transparent; + + &::-webkit-progress-bar { + background: transparent; + } + + // Those rules need to be entirely separate or they won't work, hence the + // duplication + &::-moz-progress-bar { + border-radius: 4px; + background: darken($ui-primary-color, 5%); + } + + &::-ms-fill { + border-radius: 4px; + background: darken($ui-primary-color, 5%); + } + + &::-webkit-progress-value { + border-radius: 4px; + background: darken($ui-primary-color, 5%); + } + } + &__option { position: relative; display: flex; diff --git a/app/javascript/styles/mastodon/statuses.scss b/app/javascript/styles/mastodon/statuses.scss index 19ce0ab8f..0b7be7afd 100644 --- a/app/javascript/styles/mastodon/statuses.scss +++ b/app/javascript/styles/mastodon/statuses.scss @@ -128,6 +128,16 @@ .embed, .public-layout { + .status__content[data-spoiler=folded] { + .e-content { + display: none; + } + + p:first-child { + margin-bottom: 0; + } + } + .detailed-status { padding: 15px; } @@ -159,5 +169,12 @@ .video-player { margin-top: 10px; } + + &__action-bar-button { + font-size: 18px; + width: 23.1429px; + height: 23.1429px; + line-height: 23.15px; + } } } diff --git a/app/views/about/show.html.haml b/app/views/about/show.html.haml index e0ec98ec9..07e06100a 100644 --- a/app/views/about/show.html.haml +++ b/app/views/about/show.html.haml @@ -68,11 +68,11 @@ .hero-widget__footer__column %h4= t 'about.server_stats' - %div{ style: 'display: flex' } - .hero-widget__counter{ style: 'width: 50%' } + .hero-widget__counters__wrapper + .hero-widget__counter %strong= number_to_human @instance_presenter.user_count, strip_insignificant_zeros: true %span= t 'about.user_count_after', count: @instance_presenter.user_count - .hero-widget__counter{ style: 'width: 50%' } + .hero-widget__counter %strong= number_to_human @instance_presenter.active_user_count, strip_insignificant_zeros: true %span = t 'about.active_count_after' diff --git a/app/views/accounts/_moved.html.haml b/app/views/accounts/_moved.html.haml index a82f277b1..4f71b062d 100644 --- a/app/views/accounts/_moved.html.haml +++ b/app/views/accounts/_moved.html.haml @@ -9,8 +9,10 @@ = link_to ActivityPub::TagManager.instance.url_for(moved_to_account), class: 'detailed-status__display-name p-author h-card', target: '_blank', rel: 'me noopener noreferrer' do .detailed-status__display-avatar .account__avatar-overlay - .account__avatar-overlay-base{ style: "background-image: url('#{moved_to_account.avatar.url(:original)}')" } - .account__avatar-overlay-overlay{ style: "background-image: url('#{account.avatar.url(:original)}')" } + .account__avatar-overlay-base + = image_tag moved_to_account.avatar_static_url + .account__avatar-overlay-overlay + = image_tag account.avatar_static_url %span.display-name %bdi diff --git a/app/views/application/_card.html.haml b/app/views/application/_card.html.haml index 808dce514..e7ecfecd9 100644 --- a/app/views/application/_card.html.haml +++ b/app/views/application/_card.html.haml @@ -9,7 +9,6 @@ = image_tag account.avatar.url, alt: '', width: 48, height: 48, class: 'u-photo' .display-name - %span{ id: "default_account_display_name", style: "display: none" }= account.username %bdi %strong.emojify.p-name= display_name(account, custom_emojify: true) %span diff --git a/app/views/auth/registrations/new.html.haml b/app/views/auth/registrations/new.html.haml index bcd66fb8a..457bc1d23 100644 --- a/app/views/auth/registrations/new.html.haml +++ b/app/views/auth/registrations/new.html.haml @@ -8,8 +8,8 @@ = render 'shared/error_messages', object: resource - if @invite.present? && @invite.autofollow? - .fields-group{ style: 'margin-bottom: 30px' } - %p.hint{ style: 'text-align: center' }= t('invites.invited_by') + .fields-group.invited-by + %p.hint= t('invites.invited_by') = render 'application/card', account: @invite.user.account = f.simple_fields_for :account do |ff| diff --git a/app/views/auth/sessions/two_factor.html.haml b/app/views/auth/sessions/two_factor.html.haml index 4e6bbd7a9..b2e36f6bc 100644 --- a/app/views/auth/sessions/two_factor.html.haml +++ b/app/views/auth/sessions/two_factor.html.haml @@ -2,7 +2,7 @@ = t('auth.login') = simple_form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f| - %p.hint{ style: 'margin-bottom: 25px' }= t('simple_form.hints.sessions.otp') + %p.hint.otp-hint= t('simple_form.hints.sessions.otp') .fields-group = f.input :otp_attempt, type: :number, wrapper: :with_label, label: t('simple_form.labels.defaults.otp_attempt'), input_html: { 'aria-label' => t('simple_form.labels.defaults.otp_attempt'), :autocomplete => 'off' }, autofocus: true diff --git a/app/views/directories/index.html.haml b/app/views/directories/index.html.haml index 333a257e1..bdc1e9d5a 100644 --- a/app/views/directories/index.html.haml +++ b/app/views/directories/index.html.haml @@ -28,7 +28,6 @@ = image_tag account.avatar.url, alt: '', width: 48, height: 48, class: 'u-photo' .display-name - %span{ id: "default_account_display_name", style: "display: none" }= account.username %bdi %strong.emojify.p-name= display_name(account, custom_emojify: true) %span= acct(account) diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 747d80bf0..25d001337 100755 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -34,6 +34,6 @@ %body{ class: body_classes } = content_for?(:content) ? yield(:content) : yield - %div{ style: 'display: none'} + .logo-resources = render file: Rails.root.join('app', 'javascript', 'images', 'logo_transparent.svg') = render file: Rails.root.join('app', 'javascript', 'images', 'logo_full.svg') diff --git a/app/views/layouts/embedded.html.haml b/app/views/layouts/embedded.html.haml index d7e74ade5..4a40b8584 100644 --- a/app/views/layouts/embedded.html.haml +++ b/app/views/layouts/embedded.html.haml @@ -19,5 +19,5 @@ %body.embed = yield - %div{ style: 'display: none'} + .logo-resources = render file: Rails.root.join('app', 'javascript', 'images', 'logo_transparent.svg') diff --git a/app/views/settings/preferences/appearance/show.html.haml b/app/views/settings/preferences/appearance/show.html.haml index acddf7884..10fff406e 100644 --- a/app/views/settings/preferences/appearance/show.html.haml +++ b/app/views/settings/preferences/appearance/show.html.haml @@ -12,8 +12,8 @@ = f.input :setting_theme, collection: Themes.instance.names, label_method: lambda { |theme| I18n.t("themes.#{theme}", default: theme) }, wrapper: :with_label, include_blank: false, hint: false - unless I18n.locale == :en - .flash-message{ style: "text-align: unset; color: unset" } - #{t 'appearance.localization.body'} #{content_tag(:a, t('appearance.localization.guide_link_text'), href: t('appearance.localization.guide_link'), target: "_blank", rel: "noopener", style: "text-decoration: underline")} + .flash-message.translation-prompt + #{t 'appearance.localization.body'} #{content_tag(:a, t('appearance.localization.guide_link_text'), href: t('appearance.localization.guide_link'), target: "_blank", rel: "noopener")} %h4= t 'appearance.advanced_web_interface' diff --git a/app/views/settings/profiles/show.html.haml b/app/views/settings/profiles/show.html.haml index 6497824c6..4885878f0 100644 --- a/app/views/settings/profiles/show.html.haml +++ b/app/views/settings/profiles/show.html.haml @@ -9,7 +9,7 @@ .fields-row .fields-row__column.fields-group.fields-row__column-6 - = f.input :display_name, wrapper: :with_label, input_html: { maxlength: 30 }, hint: false + = f.input :display_name, wrapper: :with_label, input_html: { maxlength: 30, data: { default: @account.username } }, hint: false = f.input :note, wrapper: :with_label, input_html: { maxlength: 500 }, hint: false .fields-row diff --git a/app/views/statuses/_detailed_status.html.haml b/app/views/statuses/_detailed_status.html.haml index 021390e47..544b92330 100644 --- a/app/views/statuses/_detailed_status.html.haml +++ b/app/views/statuses/_detailed_status.html.haml @@ -15,12 +15,12 @@ = account_action_button(status.account) - .status__content.emojify< + .status__content.emojify{ :data => ({ spoiler: current_account&.user&.setting_expand_spoilers ? 'expanded' : 'folded' } if status.spoiler_text?) }< - if status.spoiler_text? - %p{ :style => ('margin-bottom: 0' unless current_account&.user&.setting_expand_spoilers) }< + %p< %span.p-summary> #{Formatter.instance.format_spoiler(status, autoplay: autoplay)}  %button.status__content__spoiler-link= t('statuses.show_more') - .e-content{ style: "display: #{!current_account&.user&.setting_expand_spoilers && status.spoiler_text? ? 'none' : 'block'}; direction: #{rtl_status?(status) ? 'rtl' : 'ltr'}" } + .e-content{ dir: rtl_status?(status) ? 'rtl' : 'ltr' } = Formatter.instance.format(status, custom_emojify: true, autoplay: autoplay) - if status.preloadable_poll = react_component :poll, disabled: true, poll: ActiveModelSerializers::SerializableResource.new(status.preloadable_poll, serializer: REST::PollSerializer, scope: current_user, scope_name: :current_user).as_json do diff --git a/app/views/statuses/_poll.html.haml b/app/views/statuses/_poll.html.haml index de5357e6d..64e62e97c 100644 --- a/app/views/statuses/_poll.html.haml +++ b/app/views/statuses/_poll.html.haml @@ -10,13 +10,15 @@ - percent = total_votes_count > 0 ? 100 * option.votes_count / total_votes_count : 0 %label.poll__option>< %span.poll__number>< - - if own_votes.include?(index) - %i.poll__voted__mark.fa.fa-check = "#{percent.round}%" %span.poll__option__text = Formatter.instance.format_poll_option(status, option, autoplay: autoplay) + - if own_votes.include?(index) + %span.poll__voted + %i.poll__voted__mark.fa.fa-check - %span.poll__chart{ style: "width: #{percent}%" } + %progress{ max: 100, value: percent < 1 ? 1 : percent, 'aria-hidden': 'true' } + %span.poll__chart - else %label.poll__option>< %span.poll__input{ class: poll.multiple? ? 'checkbox' : nil}>< diff --git a/app/views/statuses/_simple_status.html.haml b/app/views/statuses/_simple_status.html.haml index 5d16b1fc2..ff09ab2ee 100644 --- a/app/views/statuses/_simple_status.html.haml +++ b/app/views/statuses/_simple_status.html.haml @@ -19,12 +19,12 @@ %span.display-name__account = acct(status.account) = fa_icon('lock') if status.account.locked? - .status__content.emojify< + .status__content.emojify{ :data => ({ spoiler: current_account&.user&.setting_expand_spoilers ? 'expanded' : 'folded' } if status.spoiler_text?) }< - if status.spoiler_text? - %p{ :style => ('margin-bottom: 0' unless current_account&.user&.setting_expand_spoilers) }< + %p< %span.p-summary> #{Formatter.instance.format_spoiler(status, autoplay: autoplay)}  %button.status__content__spoiler-link= t('statuses.show_more') - .e-content{ style: "display: #{!current_account&.user&.setting_expand_spoilers && status.spoiler_text? ? 'none' : 'block'}; direction: #{rtl_status?(status) ? 'rtl' : 'ltr'}" } + .e-content{ dir: rtl_status?(status) ? 'rtl' : 'ltr' } = Formatter.instance.format(status, custom_emojify: true, autoplay: autoplay) - if status.preloadable_poll = react_component :poll, disabled: true, poll: ActiveModelSerializers::SerializableResource.new(status.preloadable_poll, serializer: REST::PollSerializer, scope: current_user, scope_name: :current_user).as_json do @@ -51,18 +51,18 @@ .status__action-bar .status__action-bar__counter - = link_to remote_interaction_path(status, type: :reply), class: 'status__action-bar-button icon-button modal-button', style: 'font-size: 18px; width: 23.1429px; height: 23.1429px; line-height: 23.15px;' do + = link_to remote_interaction_path(status, type: :reply), class: 'status__action-bar-button icon-button modal-button' do - if status.in_reply_to_id.nil? = fa_icon 'reply fw' - else = fa_icon 'reply-all fw' .status__action-bar__counter__label= obscured_counter status.replies_count - = link_to remote_interaction_path(status, type: :reblog), class: 'status__action-bar-button icon-button modal-button', style: 'font-size: 18px; width: 23.1429px; height: 23.1429px; line-height: 23.15px;' do + = link_to remote_interaction_path(status, type: :reblog), class: 'status__action-bar-button icon-button modal-button' do - if status.distributable? = fa_icon 'retweet fw' - elsif status.private_visibility? || status.limited_visibility? = fa_icon 'lock fw' - else = fa_icon 'envelope fw' - = link_to remote_interaction_path(status, type: :favourite), class: 'status__action-bar-button icon-button modal-button', style: 'font-size: 18px; width: 23.1429px; height: 23.1429px; line-height: 23.15px;' do + = link_to remote_interaction_path(status, type: :favourite), class: 'status__action-bar-button icon-button modal-button' do = fa_icon 'star fw'