Add admin pages to the mobile version #7295
Conversation
| @@ -0,0 +1,70 @@ | |||
|
|
|||
| %li.user.media | |||
| %div.pull-left | |||
There was a problem hiding this comment.
%div.pull-left can be written as .pull-left since %div is implicit
| %span.media-object | ||
| = person_image_tag(user.person, size: :thumb_small) | ||
|
|
||
| %div.media-body.row |
There was a problem hiding this comment.
%div.media-body.row can be written as .media-body.row since %div is implicit
| = person_image_tag(user.person, size: :thumb_small) | ||
|
|
||
| %div.media-body.row | ||
| %div.pull-right |
There was a problem hiding this comment.
%div.pull-right can be written as .pull-right since %div is implicit
| %div.media-body.row | ||
| %div.pull-right | ||
| %span.label.label-default | ||
| = t('.id') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| = t('.id') | ||
| = user.id | ||
| %span.label.label-info | ||
| = t('.guid') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
|
|
||
| %div | ||
| .unstyled.text-right.actions | ||
| = link_to t('admins.user_search.view_profile'), person_path(user.person), class: 'btn btn-default btn-block' |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %div | ||
| .unstyled.text-right.actions | ||
| = link_to t('admins.user_search.view_profile'), person_path(user.person), class: 'btn btn-default btn-block' | ||
| = link_to t('admins.user_search.add_invites'), add_invites_path(user.invitation_code), class: 'btn btn-info btn-block' |
There was a problem hiding this comment.
- Line is too long. [126/120]
- Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| = link_to t('admins.user_search.view_profile'), person_path(user.person), class: 'btn btn-default btn-block' | ||
| = link_to t('admins.user_search.add_invites'), add_invites_path(user.invitation_code), class: 'btn btn-info btn-block' | ||
| - unless user.person.closed_account | ||
| = link_to t('admins.user_search.close_account'), admin_close_account_path(user), method: :post, data: { confirm: t('admins.user_search.are_you_sure') }, class: 'btn btn-danger btn-block' |
There was a problem hiding this comment.
- Line is too long. [196/120]
- Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
- Space inside { detected.
- Space inside } detected.
| = link_to t('admins.user_search.close_account'), admin_close_account_path(user), method: :post, data: { confirm: t('admins.user_search.are_you_sure') }, class: 'btn btn-danger btn-block' | ||
|
|
||
| - unless user.closed_account? | ||
| - unless user.access_locked? |
There was a problem hiding this comment.
Do not use unless with else. Rewrite these with the positive case first.
|
|
||
| - unless user.closed_account? | ||
| - unless user.access_locked? | ||
| = link_to t('admins.user_search.lock_account'), admin_lock_account_path(user), method: :post, data: { confirm: t('admins.user_search.are_you_sure_lock_account') }, class: 'btn btn-danger btn-block' |
There was a problem hiding this comment.
- Line is too long. [209/120]
- Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
- Space inside { detected.
- Space inside } detected.
| - unless user.access_locked? | ||
| = link_to t('admins.user_search.lock_account'), admin_lock_account_path(user), method: :post, data: { confirm: t('admins.user_search.are_you_sure_lock_account') }, class: 'btn btn-danger btn-block' | ||
| - else | ||
| = link_to t('admins.user_search.unlock_account'), admin_unlock_account_path(user), method: :post, data: { confirm: t('admins.user_search.are_you_sure_unlock_account') }, class: 'btn btn-danger btn-block' |
There was a problem hiding this comment.
- Line is too long. [215/120]
- Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
- Space inside { detected.
- Space inside } detected.
| - else | ||
| = link_to t('admins.user_search.unlock_account'), admin_unlock_account_path(user), method: :post, data: { confirm: t('admins.user_search.are_you_sure_unlock_account') }, class: 'btn btn-danger btn-block' | ||
|
|
||
| %div.row |
There was a problem hiding this comment.
%div.row can be written as .row since %div is implicit
| = link_to t('admins.user_search.unlock_account'), admin_unlock_account_path(user), method: :post, data: { confirm: t('admins.user_search.are_you_sure_unlock_account') }, class: 'btn btn-danger btn-block' | ||
|
|
||
| %div.row | ||
| %div.col-md-5 |
There was a problem hiding this comment.
%div.col-md-5 can be written as .col-md-5 since %div is implicit
| %div.row | ||
| %div.col-md-5 | ||
| %dl.dl-horizontal | ||
| %dt= t('username') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %dl.dl-horizontal | ||
| %dt= t('username') | ||
| %dd= user.username | ||
| %dt= t('.email') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %dd= user.username | ||
| %dt= t('.email') | ||
| %dd= user.email | ||
| %dt= t('.diaspora_handle') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %dd= user.email | ||
| %dt= t('.diaspora_handle') | ||
| %dd= user.person.diaspora_handle | ||
| %dt= t('.last_seen') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %dt= t('.diaspora_handle') | ||
| %dd= user.person.diaspora_handle | ||
| %dt= t('.last_seen') | ||
| %dd= user.last_seen || t('.unknown') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %dd= user.person.diaspora_handle | ||
| %dt= t('.last_seen') | ||
| %dd= user.last_seen || t('.unknown') | ||
| -if user.invited_by.present? |
There was a problem hiding this comment.
The - symbol should have one space separating it from code
| %dt= t('.last_seen') | ||
| %dd= user.last_seen || t('.unknown') | ||
| -if user.invited_by.present? | ||
| %dt= t('.invite_token') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| -if user.invited_by.present? | ||
| %dt= t('.invite_token') | ||
| %dd= invite_code_url(user.invited_by.invitation_code) | ||
| %dt= t('.account_closed') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %dt= t('.account_closed') | ||
| %dd | ||
| - if user.person.closed_account | ||
| %span.label.label-warning= t('.yes') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| - if user.person.closed_account | ||
| %span.label.label-warning= t('.yes') | ||
| - else | ||
| %span.label.label-success= t('.no') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %span.label.label-warning= t('.yes') | ||
| - else | ||
| %span.label.label-success= t('.no') | ||
| %dt= t('.nsfw') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %dt= t('.nsfw') | ||
| %dd | ||
| - if user.person.profile.nsfw | ||
| %span.label.label-warning= t('.yes') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| - if user.person.profile.nsfw | ||
| %span.label.label-warning= t('.yes') | ||
| - else | ||
| %span.label.label-success= t('.no') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| - else | ||
| %span.label.label-success= t('.no') | ||
|
|
||
| %h4= t('layouts.header.profile') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %h4= t('layouts.header.profile') | ||
|
|
||
| %dl.dl-horizontal | ||
| %dt= t('people.profile_sidebar.born') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %dl.dl-horizontal | ||
| %dt= t('people.profile_sidebar.born') | ||
| %dd= user.person.profile.birthday | ||
| %dt= t('people.profile_sidebar.gender') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| %dd= user.person.profile.birthday | ||
| %dt= t('people.profile_sidebar.gender') | ||
| %dd= user.person.profile.gender | ||
| %dt= t('people.profile_sidebar.location') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
|
#7295 (comment) this one is still not fixed |
features/mobile/drawer.feature
Outdated
| | Alice Smith | alice@alice.alice | | ||
|
|
||
| And a user with email "bob@bob.bob" is connected with "alice@alice.alice" | ||
| Given an admin with email "alice@alice.alice" |
There was a problem hiding this comment.
I think we should make alice an admin just for the navigate to the admin pages scenario. All other sections should run with alice as a normal user.
| @@ -15,6 +15,10 @@ def create_user(overrides={}) | |||
| user | |||
| end | |||
|
|
|||
| def make_admin(user) | |||
There was a problem hiding this comment.
I don't think we really need to wrap Role.add_admin(user) method in another one here
|
|
||
| = t(".amount_of", count: counter) | ||
| %br | ||
| - @created_users_by_week[selected_week].each do |m| |
There was a problem hiding this comment.
should be created_users_by_week instead of @created_users_by_week
There was a problem hiding this comment.
Doing that I can an error no implicit conversion of String into Integer, not sure how to solve it, I tried to print selected_week but it looks it is empty. Weird.
There was a problem hiding this comment.
Works for me the following way: https://gist.github.com/cmrd-senya/8378adf3ad3e5e059ae1fc1a8128f56f
There was a problem hiding this comment.
Thanks got it. I did it with only one variable. So, corrected.
|
So still a question, all other points should be resolved. |
app/assets/stylesheets/admin.scss
Outdated
| .pull-right .label{ display: inline-block; } | ||
| } | ||
| } | ||
|
|
||
| .invite-emails { |
There was a problem hiding this comment.
I think this should go below the /** Invites panel **/ comment
app/views/admins/_stats.haml
Outdated
| :ruby | ||
| model = instance_variable_get("@#{name}") | ||
| count = model[:yesterday] | ||
| name = name == :aspect_memberships ? t(".shares", count: count) : t(".#{name}", count: count) |
There was a problem hiding this comment.
Could you please pick a different name for the variable on the left side insead of name? Reusing the same name looks confusing. Also this new variable has a different meaning, it's rather a label with count. Maybe it should be called label_with_count or heading_counter or count_label, or something like this?
app/views/admins/_stats.haml
Outdated
|
|
||
| .col-md-3 | ||
| %h2{style: "font-weight:bold;"} | ||
| = name.to_s |
There was a problem hiding this comment.
This doesn't need to_s here, it is already a string
app/views/admins/_stats.haml
Outdated
| = name.to_s | ||
| %h4 | ||
| = model[:day_before] | ||
| %span.percent_change{class: (model[:change] > 0 ? "green" : "red")} |
There was a problem hiding this comment.
Actually these "green" and "red" classes don't exist, so I think we should either remove them or implement
| %dd= user.email | ||
| %dt= t('.diaspora_handle') | ||
| %dt= t(".diaspora_handle") |
There was a problem hiding this comment.
We were moving away from calling this a "diaspora handle" to a name "diaspora id". It's not necessary to change the translation ID here, but maybe we may want to change the English translation actually? @SuperTux88 what do you think?
I'm talking about this label:
There was a problem hiding this comment.
Yes, the key shouldn't be changed (otherwise we need to re-translate it), but the translation should be "diaspora* ID".
|
Remarks fixed, thanks for your review! |
|
I just had a look at this (because I wanted to check if I can merge it to 0.6.7.0), and I think the code is OK and it works. There are probably things that can be done better (for example I'd like when the admin section stays open in the drawer when at an admin page, so I don't need to open it every time), but this can always be improved in a follow up. So it's OK for now (this PR is already big enough), and I don't want to polish mobile admin that much anyway, because I think we should focus on making the desktop view more responsive. But there are really many commits, so as @cmrd-senya wrote, it would be good if you can squash some of them (so fix pronto stuff and review remarks in the commit where it belongs). So can you try to squash some of them, because when trying to find where some lines come from and why they are the way they are and then find something like "Fix remarks" with some random changes in it doesn't help ;) When it's too complicated and you have many conflicts, then just let it as it is, instead of breaking everything when resolving conflicts. |
|
I was waiting for a final "Everything is fine" to squash ;) I'll do it today. |
Flaburgan
force-pushed
the
report-mobile
branch
2 times, most recently
from
da20b2e
to
2e9a812
Compare
Indicates when there is no report to review to avoid a weird empty page
…responsive code to use bootstrap grid Hide pod version alert only in the mobile version, not depending on screen size
|
Alright, commits squashed from 28 down to 7. Please tell me if that's enough. |
|
I just realized that this PR doesn't deal with the moderator role. I guess it's not a blocker I'll do it in another one. There is no bug, the drawer doesn't display anything. |
|
Merged as 07d0ed9, 427aa87, 2782edc, a7d97b7, 7bdf33e, 5cb4c6e and 276b640 Thank you @SansPseudoFix and @Flaburgan and also thank you @cmrd-senya for the reviews ❤️ |
I was starting to code the report view, but I figured out I have to code all admin pages, so…
This is a early-PR because I don't want to code every pages and finally understand it won't be merge or wanted.
So far, reports page, user-search and user entry pages are mobile friendly (doesn't mean totally finished though). I also add admin menu to the drawer.
Here some screenshots:
What do you think? Do I continue or not?
Next:
sidekiq monitor?already mobile friendlyEdit: and, of course, I forgot my "damit, I forgot pronto" commit… Sorry.