Skip to content

Conversation

godplayer56
Copy link
Contributor

I have updated the dashboard file by introducing the mozilla logo and have also updated the typography to use the mozilla-protocol

This PR is for the issue #2976

I request a review and suggestions for this PR @gabrielbusta @gbrownmozilla .

@godplayer56
Copy link
Contributor Author

godplayer56 commented Oct 16, 2023

Hey @bhearsum @gabrielbusta . I was trying to fix the ui-build error which got resolved after I removed the group parameter but I am not able to fix the error with ui-tests.

It shows the same error when I try to run docker exec balrog-balrogui-1 yarn lint --fix for fixing lint errors, but it just displays the errors without correcting them.

How should I resolve these errors? Plus should I do some other changes to update the ui?

@@ -35,7 +36,25 @@ export default createTheme({
},
},
typography: {
Copy link
Contributor

@gabrielbusta gabrielbusta Oct 17, 2023

Choose a reason for hiding this comment

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

ui/.eslintrc.js Outdated
@@ -1,3 +1,3 @@
const neutrino = require('neutrino');

module.exports = neutrino().eslintrc();
module.exports = neutrino().eslintrc();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you revert the changes to this file?

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 changed the file accidently in a commit 😅 , I'll revert the changes

Copy link
Contributor

@gabrielbusta gabrielbusta left a comment

Choose a reason for hiding this comment

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

Updating the color palette and removing balrog.svg from the background will go a long way in making balrog look more in line with Mozilla Protocol

@godplayer56
Copy link
Contributor Author

Updating the color palette and removing balrog.svg from the background will go a long way in making balrog look more in line with Mozilla Protocol

Yes I agree! The color palette would look nicer and in line with the protocol. I'll update the theme to use the protocol palette.

@godplayer56
Copy link
Contributor Author

@gabrielbusta I have removed the balrog.svg file from the background and updated the pallette, do we need anymore changes?

@godplayer56
Copy link
Contributor Author

Hey @gabrielbusta just a follow up on this. 😅

Copy link
Contributor

@gabrielbusta gabrielbusta left a comment

Choose a reason for hiding this comment

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

It looks good to me @godplayer56

Just one more question before I land it. I am not sure what deleting some of the pallete styles does. Should we update those to protocol colors or leave them as is?

},
error: red,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be updated? What changes when you delete these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I delete them the theme changes to the original theme it was. for example the dashboard becomes blue when I delete the palette

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh OK I see. Can we pick similar colors from the protocol pallette?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we do have similar colours in the protocol pallette, we can pick them if they look more lively. Should I update the pallette to include those colours?

Copy link
Contributor

@gabrielbusta gabrielbusta left a comment

Choose a reason for hiding this comment

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

I can't see the Logo or the pages' {title}. The Logo component should be modified for use here. balrog doesn't have a group concept like shipit. You may be able to display different protocol logos based on some value in the Dashboard component's props. Or you could simplify the Logo component to always show the Mozilla "m".

@godplayer56
Copy link
Contributor Author

I can't see the Logo or the pages' {title}. The Logo component should be modified for use here. balrog doesn't have a group concept like shipit. You may be able to display different protocol logos based on some value in the Dashboard component's props. Or you could simplify the Logo component to always show the Mozilla "m".

Ok I'll update the logo component 👍

@godplayer56
Copy link
Contributor Author

It looks good to me @godplayer56

Just one more question before I land it. I am not sure what deleting some of the pallete styles does. Should we update those to protocol colors or leave them as is?

deleting them reverts the theme to what it was before, I guess it would be better if we update those to protocol colors.

What colors should we use as protocol colors?

@godplayer56
Copy link
Contributor Author

Screenshot from 2023-10-25 01-44-39
@gabrielbusta This is what I see when I start the docker containers. When I inspect the webpage I see that the logo is present but there is some issue with its rendering. Can you provide me with suggestions on what I should do to resolve this?(I have updated the logo component by removing the groups concept)

@godplayer56
Copy link
Contributor Author

@gabrielbusta I have updated the code to include the mozilla family logo as the simple "m" logo wasn't rendering maybe because of the pallette colours we have chosen 😅 . Also should I update the pallette and to what colours?

@Leo-ainy
Copy link

Leo-ainy commented Jan 20, 2024

Enhancing Balrog's visual alignment with the Mozilla Protocol involves a significant improvement by updating the color palette and eliminating the background's balrog.svg. This step will contribute to a more cohesive and harmonious appearance.
https://github.com/mozilla-releng

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.

3 participants