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.

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

[FIX] Remove sidebar header on admin embedded version #8334

Merged
merged 2 commits into from
Sep 29, 2017

Conversation

karlprieb
Copy link
Contributor

@RocketChat/core

Closes #8333

screenshot-2017-9-27 1 rocket chat

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-8334 September 27, 2017 19:05 Inactive
@@ -6,5 +6,9 @@ Template.burger.helpers({
if (Session.equals('isMenuOpen', true)) {
return 'menu-opened';
}
},
embeddedVersion() {
console.log('burger', RocketChat.Layout.isEmbedded());

Choose a reason for hiding this comment

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

Ops!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry :(

<div class="rooms-list">
{{#unless embeddedVersion}}
<header class="sidebar__header">
{{> accountBox lighten=true modifier="--account" }}
Copy link
Member

Choose a reason for hiding this comment

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

Remove this account box

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can remove that, but I won't do that on this PR. This PR is about embedded view and not about accountBox.

<div class="rooms-list" aria-label="{{_ "Administration"}}">
{{#unless embeddedVersion}}
<header class="sidebar__header">
{{> accountBox lighten=true modifier="--admin"}}
Copy link
Member

Choose a reason for hiding this comment

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

Also remove this account box

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can remove that, but I won't do that on this PR. This PR is about embedded view and not about accountBox.

{{> accountBox lighten=true modifier="--admin"}}
<button class="sidebar-flex__back-button" data-action="back">
{{> icon block="sidebar-flex__back-icon" icon="back"}}
<span>{{_ "Back_to_chat"}}</span>
Copy link
Member

Choose a reason for hiding this comment

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

Change "Back_to_chat" to just "Back"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why?

Copy link
Member

Choose a reason for hiding this comment

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

Less is More

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, it's easy to maintain translations :)

{{> accountBox lighten=true modifier="--account" }}
<button class="sidebar-flex__back-button" data-action="back">
{{> icon block="sidebar-flex__back-icon" icon="back"}}
<span>{{_ "Back_to_chat"}}</span>
Copy link
Member

Choose a reason for hiding this comment

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

Change "Back_to_chat" to just "Back"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why?

@rodrigok rodrigok dismissed engelgabriel’s stale review September 29, 2017 12:37

Will be done in another PR

@rodrigok rodrigok merged commit 5abb1d2 into develop Sep 29, 2017
@rodrigok rodrigok deleted the admin-embedded-version branch September 29, 2017 12:38
rodrigok added a commit that referenced this pull request Sep 29, 2017
[FIX] Remove sidebar header on admin embedded version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support embedded layout on /admin URL for mobile apps
4 participants