Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Onboarding] Colours need updating #7606

Closed
AmyYLee opened this issue Jan 10, 2020 · 36 comments
Closed

[Onboarding] Colours need updating #7606

AmyYLee opened this issue Jan 10, 2020 · 36 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified eng:ready Ready for engineering Feature:Onboarding First Run, Contextual Feature Recommendation/Recommender CFR help wanted Help wanted from a contributor. More complex than good first issue. implementation review

Comments

@AmyYLee
Copy link
Collaborator

AmyYLee commented Jan 10, 2020

Hi,

Please reference mock below for updated colours. Thanks

Onboarding spec 2

┆Issue is synchronized with this Jira Task

@AmyYLee AmyYLee added 🐞 bug Crashes, Something isn't working, .. Feature:Onboarding First Run, Contextual Feature Recommendation/Recommender CFR implementation review needs:group-triage labels Jan 10, 2020
@AmyYLee AmyYLee changed the title [Onboarding] Colours are wrong [Onboarding] Colours need updating Jan 10, 2020
@ekager
Copy link
Contributor

ekager commented Jan 10, 2020

@apbitner mentioned a different color for the light theme cards here: #7471 (comment)

Could we confirm which color we are going with for these cards?

@apbitner
Copy link

@ekager Sorry for any confusion, Amy's spec above is correct

@boek boek added help wanted Help wanted from a contributor. More complex than good first issue. eng:ready Ready for engineering and removed needs:group-triage labels Jan 14, 2020
@boek boek added this to Polish Bugs to Triage in Feature Polish via automation Jan 14, 2020
@boek boek moved this from Polish Bugs to Triage to UX Implementation Review in Feature Polish Jan 14, 2020
@joshvocal
Copy link
Contributor

Could I take this one?

@ekager
Copy link
Contributor

ekager commented Jan 17, 2020

Sure @joshvocal !

@joshvocal
Copy link
Contributor

If anyone else reading this wants to take this issue, you can take it instead of me.

@ghost
Copy link

ghost commented Jan 22, 2020

I would like to work on this. Can you assign this task to me @ekager?

@ghost
Copy link

ghost commented Feb 1, 2020

@AmyYLee Do you have drawable assets for the unselected theme images like the one shown below (light and dark theme variants)?
Screenshot from 2020-02-01 16-35-00

@AmyYLee
Copy link
Collaborator Author

AmyYLee commented Feb 3, 2020

@AmyYLee Do you have drawable assets for the unselected theme images like the one shown below (light and dark theme variants)?
Screenshot from 2020-02-01 16-35-00

@tvenissat Hi, can you be more specific? Which asset do you need and in what format? Thanks

@apbitner
Copy link

apbitner commented Feb 5, 2020

@tvenissat I provided these assets for another contributor on #6980, please let me know if you need anything different

Illustration - Toolbar Positions.zip

@lime124 lime124 added this to Onboarding in Polish Feb 6, 2020
@lime124 lime124 removed this from UX Implementation Review in Feature Polish Feb 6, 2020
@ghost
Copy link

ghost commented Feb 8, 2020

@AmyYLee @apbitner Sorry for taking so long. The assets in that zip file are not the assets needed. To be clear, I need this asset recolored with a grey border and this asset recolored with a grey border. The two assets I linked are currently used to represent the active theme since they have colored borders. I need two more assets with grey borders to represent the inactive theme.

@mcarare
Copy link
Contributor

mcarare commented Feb 10, 2020

@tvenissat The assets do not contain the radio button. You do not need to change the image, just the radio button colour for selected.

@ghost
Copy link

ghost commented Feb 10, 2020

@mcarare

The assets do not contain the radio button.

I never mentioned the radio button. The radio button is just shown in the design document and I didn't remove it (I'm not too familiar with image editing). The border color in the image is not done programmatically but is a part of the image itself. I can change the radio button color programmatically but I cannot change the border of the image without being provided with the asset.

Screenshot from 2020-02-10 09-52-17

You are correct regarding this image. Changing the enabled radio button color to match the image border color, which appears to be #5A30C5, is all that needs when the light theme is enabled by the user. I need the same asset with a grey border to represent the light theme not being enabled by the user, as the image below demonstrates.

Screenshot from 2020-02-10 09-52-26

The problem is that this asset is not in the repository as far as I can tell. I need the theme image with a grey border to represent the disabled state. I need this for both light and dark theme images as specified in the design document.

I may be misunderstanding the design document but I think these 2 assets are necessary.

@apbitner
Copy link

@tvenissat I believe you are correct, however, it appears the gray border was never implemented for the Theme onboarding card illustrations so let's not worry about it for now. I would rather just be consistent with the other card which appears to only use purple borders.

@ghost
Copy link

ghost commented Feb 11, 2020

@apbitner Sounds good.

@ghost
Copy link

ghost commented Feb 13, 2020

@AmyYLee What is the radio button color used here?
Screenshot from 2020-02-13 17-00-39

BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 4, 2020
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 6, 2020
…onboarding illustrations

RTL for toolbar are still missing
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 6, 2020
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 6, 2020
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 6, 2020
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 6, 2020
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 6, 2020
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 6, 2020
BranescuMihai added a commit that referenced this issue May 6, 2020
@BranescuMihai BranescuMihai added eng:qa:needed QA Needed and removed 🙅 waiting Issues that are blocked or has dependencies that are not ready labels May 8, 2020
@lobontiumira
Copy link

Verified on the latest Nightly build from 5/8 with Sony Xperia Z5 Premium (Android 7.1.1), and OnePlus 5T (Android 9).

The colors are updated on the following:

  • the selected/unselected radio button matches the border outlines, both on RTL languages, and LTR languages,
  • the background of all cards matches the collection's background, both on RTL languages, and LTR languages. Please see the screenshots below:

LTR

RTL

  • @AmyYLee the "Read our privacy notice" button still has a background color, as it can be seen in the screenshot below:

read

I'll remove the qa:needed label, but not close the issue yet.

@lobontiumira lobontiumira removed the eng:qa:needed QA Needed label May 8, 2020
@BranescuMihai
Copy link
Contributor

@softvision-miralobontiu I used this abstract to guide myself, there the background is still included. Also there's a comment from Amy above that specifies the new button backgrounds.

@AmyYLee should the card buttons have backgrounds?

@sblatz sblatz added this to Onboarding in Sawyer's Features May 13, 2020
@AmyYLee
Copy link
Collaborator Author

AmyYLee commented May 14, 2020

@softvision-miralobontiu I used this abstract to guide myself, there the background is still included. Also there's a comment from Amy above that specifies the new button backgrounds.

@AmyYLee should the card buttons have backgrounds?

Hi @BranescuMihai

Please follow the abstract document for reference for colours of buttons and backgrounds.
I had mentioned to remove the button background colour earlier because our card colour wasn't white at the time but this is no longer the case. Thanks.

@lobontiumira
Copy link

Considering AmyLee's comment from above, I will close this issue as verified as fixed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified eng:ready Ready for engineering Feature:Onboarding First Run, Contextual Feature Recommendation/Recommender CFR help wanted Help wanted from a contributor. More complex than good first issue. implementation review
Projects
No open projects
Development

No branches or pull requests

10 participants