Service section design changes #474

muzammalrahim · 2021-02-01T15:26:18Z

Proposed changes

Checklist

Lint and unit tests pass locally with my changes.
I have added tests that prove my fix is effective or that my feature works.
I have added documentation to describe my feature.
I have squashed my commits into logic units.
I have described the changes in the commit messages.

Other information

…esign_changes # Conflicts: # weblate_web/static/style-rtl.css # weblate_web/static/style.css

accesslint

There are accessibility issues in these changes.

weblate_web/templates/snippets/service.html

accesslint · 2021-02-01T15:27:22Z

weblate_web/templates/snippets/service.html

+                    <div class="user-img"><img src="{% static "img/Shield.png" %}"></div>
+                    {% else %}
+                    {% csrf_token %}
+                    <img onclick="document.getElementById('server_user_{{owner.id}}_form').submit()" class="user-img" name="remove" value="remove" src="{% static "img/delete.png" %}">


This image is missing a text alternative (alt attribute). This is a problem for people using screen readers.

accesslint · 2021-02-01T15:27:22Z

weblate_web/templates/snippets/service.html

+                    {% endfor %}
+                    <form method="post" action="{% url 'service-user' pk=service.pk %}">
+                    {% csrf_token %}
+                    <input type="email" placeholder="{% trans "Invite e-mail" %}" name="email" />


Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input.

weblate_web/templates/user.html

weblate_web/templates/snippets/service.html

weblate_web/static/img/delete.svg

weblate_web/static/style.css

weblate_web/templates/snippets/service.html

weblate_web/templates/user.html

weblate_web/static/style.css

accesslint

There are accessibility issues in these changes.

accesslint · 2021-02-02T11:09:44Z

weblate_web/templates/snippets/service.html

+                    <div class="user-img"><img src="{% static "img/Shield.png" %}" alt="Shield"></div>
+                    {% else %}
+                    {% csrf_token %}
+                    <img class="user-img" onclick="removeUser('{{owner.id}}')" name="remove" value="remove" src="{% static "img/delete.png" %}">


This image is missing a text alternative (alt attribute). This is a problem for people using screen readers.

accesslint · 2021-02-02T11:09:44Z

weblate_web/templates/snippets/service.html

+                    {% endfor %}
+                    <form method="post" action="{% url 'service-user' pk=service.pk %}">
+                    {% csrf_token %}
+                    <input type="email" placeholder="{% trans "Invite e-mail" %}" name="email" />


Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input.

weblate_web/templates/snippets/service.html

weblate_web/templates/user.html

weblate_web/templates/snippets/payment.html

weblate_web/templates/user.html

weblate_web/models.py

weblate_web/templates/snippets/service.html

nijel · 2021-02-05T10:39:42Z

weblate_web/static/style.css

+.donation-box-updated {
+  display: flex;
+}


Can you please just change the existing CSS instead of introducing yet another one? We want the styles to be maintainable in the long term and this is not a way to do that...

lgtm-com · 2021-02-09T07:22:41Z

This pull request introduces 1 alert when merging 4d8f211 into 72bce53 - view on LGTM.com

new alerts:

1 for Syntax error

weblate_web/templates/user.html

nijel · 2021-02-09T11:53:25Z

weblate_web/models.py

-        if (
-            not self.hosted_subscriptions.exists()
-            and not self.shared_subscriptions.exists()
-        ):


Seems you've messed up the indentation fully here. Mixing tabs and spaces makes my Python reject loading the module completely.

Okay, let me check it again.

nijel · 2021-02-09T11:54:55Z

weblate_web/static/style-rtl.css

@@ -4032,71 +4033,501 @@ input.fullwidth {
  }
 }

-/* Your Weblate Team - About Us */
+.testimonial {


Please remove testimonial CSS from this branch...

nijel · 2021-02-09T11:55:54Z

weblate_web/static/style-rtl.css

+  max-height: 267px;
+  max-width: 192px;
+  float: left;
+  -webkit-transform: scaleX(-1);


Please use transform directly, it seems well supported these days (https://developer.mozilla.org/en-US/docs/Web/CSS/transform).

Okay, let me change it.

nijel · 2021-02-09T11:58:53Z

weblate_web/templates/snippets/bottom.html

@@ -1,6 +1,6 @@
 {% load i18n %}
 <section class="bottom {{ extra_css }}">
-    <div class="row">
+    <div class="row donation-box-updated">


Please revert this change and do it in a separate PR.

I have already changed that Michal, i haven't created separate PR yet. I am trying to fix the support boxes using flex and once i am done, i will create a separate PR for that.

nijel · 2021-02-09T12:00:08Z

weblate_web/static/style-rtl.css

+  padding-top: 22px !important;
+}
+
+.style6 > div {


Can we please have reasonable class names? "style6" really doesn't tell anything on how it is being used. Somebody will look at your code few years later and should be able to understand it and having descriptive names really helps in this.

Yes, you are right. I apologize for this, i will change it.

nijel · 2021-02-16T10:19:41Z

Rebasing it on current master so that it can be merged is a good start.

Also, the indentation still seems wrong...

muzammalrahim · 2021-02-16T10:29:42Z

Rebasing it on current master so that it can be merged is a good start.

Also, the indentation still seems wrong...

Can you please mention the part where indentation seems to be wrong?

nijel · 2021-02-16T10:43:08Z

In the models.py, see https://github.com/WeblateOrg/website/pull/474/files - there is a bunch of errors listed. Most of that (if not all) can be fixed by using pre-commit, it will do the correct formatting if the code is syntactically correct.

muzammalrahim · 2021-02-16T10:48:00Z

In the models.py, see https://github.com/WeblateOrg/website/pull/474/files - there is a bunch of errors listed. Most of that (if not all) can be fixed by using pre-commit, it will do the correct formatting if the code is syntactically correct.

Yes, i know. I was wondering if you want me to create a new Pull Request for those changes or do you want me to commit in this branch?

nijel · 2021-02-16T11:01:00Z

It really doesn't matter, whatever is easier for you to produce a pull request which can be merged.

muzammalrahim · 2021-02-16T11:06:11Z

It really doesn't matter, whatever is easier for you to produce a pull request which can be merged.

Okay, i got it. I will be creating a new PR which hopefully will be easier to merge.

nijel

I've just again reviewed this pull request and here is a summary of issues I see. Most of them have been already mentioned several times during the review:

Conflicts with current master branch
Changes in non-related parts of code (for example comment removal as first line of the diff)
Duplication of the testimonial CSS
Poorly named CSS classes (med1, med2, style2, style3, ...)

muzammalrahim · 2021-02-24T05:39:27Z

i am checking these issue

…esign_changes # Conflicts: # weblate_web/static/style-rtl.css # weblate_web/static/style.css # weblate_web/templates/snippets/bottom.html

codecov · 2021-03-01T11:41:18Z

Codecov Report

Merging #474 (9e30870) into master (f5d1888) will increase coverage by 0.03%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master     #474      +/-   ##
==========================================
+ Coverage   83.53%   83.57%   +0.03%     
==========================================
  Files          26       27       +1     
  Lines        2484     2490       +6     
  Branches      253      253              
==========================================
+ Hits         2075     2081       +6     
  Misses        315      315              
  Partials       94       94

nijel · 2021-03-01T11:57:49Z

Thanks. I've merged this, but I'm fixing things requested in #474 (review) in #517

muzammalrahim added 5 commits January 20, 2021 18:46

About page - added team section

cde96d7

About page - added team section

8586925

About page - added team section

eb3085f

User page - service section design changes

680b874

Merge remote-tracking branch 'upstream/master' into service_section_d…

62bf774

…esign_changes # Conflicts: # weblate_web/static/style-rtl.css # weblate_web/static/style.css

accesslint bot suggested changes Feb 1, 2021

View reviewed changes

nijel reviewed Feb 1, 2021

View reviewed changes

User page - service section design changes-update

a93891b

accesslint bot suggested changes Feb 2, 2021

View reviewed changes

nijel reviewed Feb 2, 2021

View reviewed changes

weblate_web/templates/snippets/payment.html Outdated Show resolved Hide resolved

nijel reviewed Feb 2, 2021

View reviewed changes

weblate_web/templates/user.html Outdated Show resolved Hide resolved

muzammalrahim added 2 commits February 3, 2021 15:37

User page - service section design changes-update

94d3e12

User page - service section design changes-update

558b0f5

nijel reviewed Feb 3, 2021

View reviewed changes

weblate_web/models.py Outdated Show resolved Hide resolved

weblate_web/templates/snippets/service.html Show resolved Hide resolved

User page - service section design changes-update-final fixes

9b0b8be

nijel reviewed Feb 5, 2021

View reviewed changes

Models.py localization and formatting

4d8f211

nijel reviewed Feb 9, 2021

View reviewed changes

nijel mentioned this pull request Feb 16, 2021

boxes layout update #498

Merged

4 tasks

fix indentation issue in models file

707b5b3

nijel requested changes Feb 23, 2021

View reviewed changes

AyazYousafxai added 10 commits February 24, 2021 17:17

Merge remote-tracking branch 'upstream/master' into service_section_d…

84fd340

…esign_changes # Conflicts: # weblate_web/static/style-rtl.css # weblate_web/static/style.css # weblate_web/templates/snippets/bottom.html

merge with upstream, fix indentation

8aa34cd

merge with upstream, fix indentation and css class name change

eae119e

merge with upstream, fix indentation and css class name change

5fe7e4e

extra lines remove

9f5d0ad

eslint failed

10b48e9

end-of-file-fixer

e6fbb44

end-of-file-fixer

5102608

end-of-file-fixer

909abd5

end-of-file-fixer

c362388

nijel changed the base branch from master to future March 1, 2021 11:50

nijel merged commit e88558c into WeblateOrg:future Mar 1, 2021

nijel mentioned this pull request Mar 1, 2021

User section improvements #517

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Service section design changes #474

Service section design changes #474

muzammalrahim commented Feb 1, 2021

accesslint bot left a comment

accesslint bot Feb 1, 2021

accesslint bot Feb 1, 2021

accesslint bot left a comment

accesslint bot Feb 2, 2021

accesslint bot Feb 2, 2021

nijel Feb 5, 2021

lgtm-com bot commented Feb 9, 2021

nijel Feb 9, 2021

muzammalrahim Feb 10, 2021

nijel Feb 9, 2021

muzammalrahim Feb 10, 2021

nijel Feb 9, 2021

muzammalrahim Feb 10, 2021

nijel Feb 9, 2021

muzammalrahim Feb 10, 2021

nijel Feb 9, 2021

muzammalrahim Feb 10, 2021

nijel commented Feb 16, 2021 •

edited

Loading

muzammalrahim commented Feb 16, 2021

nijel commented Feb 16, 2021

muzammalrahim commented Feb 16, 2021

nijel commented Feb 16, 2021

muzammalrahim commented Feb 16, 2021

nijel left a comment

muzammalrahim commented Feb 24, 2021

codecov bot commented Mar 1, 2021 •

edited

Loading

nijel commented Mar 1, 2021

Service section design changes #474

Service section design changes #474

Conversation

muzammalrahim commented Feb 1, 2021

Proposed changes

Checklist

Other information

accesslint bot left a comment

Choose a reason for hiding this comment

accesslint bot Feb 1, 2021

Choose a reason for hiding this comment

accesslint bot Feb 1, 2021

Choose a reason for hiding this comment

accesslint bot left a comment

Choose a reason for hiding this comment

accesslint bot Feb 2, 2021

Choose a reason for hiding this comment

accesslint bot Feb 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Feb 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nijel commented Feb 16, 2021 • edited Loading

muzammalrahim commented Feb 16, 2021

nijel commented Feb 16, 2021

muzammalrahim commented Feb 16, 2021

nijel commented Feb 16, 2021

muzammalrahim commented Feb 16, 2021

nijel left a comment

Choose a reason for hiding this comment

muzammalrahim commented Feb 24, 2021

codecov bot commented Mar 1, 2021 • edited Loading

Codecov Report

nijel commented Mar 1, 2021

nijel commented Feb 16, 2021 •

edited

Loading

codecov bot commented Mar 1, 2021 •

edited

Loading