Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 1018283 - [Follow-up 951665] Pending visual revision and adjustments of the VR call screen when in lockscreen #20054

Conversation

gtorodelvalle
Copy link
Contributor

No description provided.

@@ -439,8 +443,79 @@ var CallScreen = {
}
},

/**
* Code 'inspired in'
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of a big copy/paste, can we have shared code for this? With some parameters. @snowmantw what do you thnk?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we have only one function to share, and the main purpose of this is to update some specific UI elements, it may be difficult to isolate this function and make it general enough to become a shared library. However, I support to reduce copy/paste in our code base. I just don't know how to do that in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are already sharing lockscreen_slide.js and screen_layout.js. So we should be able to share this too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sharing code included in the forthcoming version of the patch ;)

@try-server-hook
Copy link

Continuous Integration started. Results

opacity: 0;
transition: opacity 0.3s linear;
}

#main-container {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is duplicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed ;) Thanks!

@try-server-hook
Copy link

Continuous Integration started. Results

@@ -147,13 +152,12 @@

colors: {
left: {
touchedColor: '0, 170, 204',
touchedColorStop: '178, 229, 239'
touchedColor: '255, 255, 255',
Copy link
Contributor

Choose a reason for hiding this comment

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

Are your sure you need to change these default properties? I've designed these properties can be overwritten while you initialize it in your component, like System/js/lockscreen.js or the callscreen . Unless the properties really can't satisfied your needs, please don't change them, or at least make them customizable and customize them in your component.

@try-server-hook
Copy link

Continuous Integration started. Results

@try-server-hook
Copy link

Continuous Integration started. Results

@try-server-hook
Copy link

Continuous Integration started. Results

@try-server-hook
Copy link

Continuous Integration started. Results

@try-server-hook
Copy link

Continuous Integration started. Results

@try-server-hook
Copy link

Continuous Integration started. Results

@try-server-hook
Copy link

Continuous Integration started. Results

@try-server-hook
Copy link

Continuous Integration started. Results

…nts of the VR call screen when in lockscreen
@try-server-hook
Copy link

gtorodelvalle Germán Toro del Valle (gtorodelvalle) started tests. Results

gtorodelvalle added a commit that referenced this pull request Jul 3, 2014
…ockscreen-visual-refresh-reloaded

Bug 1018283 - [Follow-up 951665] Pending visual revision and adjustments of the VR call screen when in lockscreen
@gtorodelvalle gtorodelvalle merged commit 05670c0 into mozilla-b2g:master Jul 3, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants