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

[Phase 1.5] Phase 1.5 Styles Update (Combined PR) #50040

Merged
merged 19 commits into from
Mar 15, 2023

Conversation

levadadenys
Copy link
Collaborator

@levadadenys levadadenys commented Feb 2, 2023

Phase 1.5 Styles Update (Combined PR)

One big PR for Phase 1.5 style updates. (See attached screenshots).

Current updates implemented:

Also updates most of the Labs and Codeworkspace (except Minecraft).

Includes following PRs:
#50095
#50233
#50621
#50703
#50676

Before:
image
image

After:
image
image

Links

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@levadadenys levadadenys requested review from kelbyhawn and a team February 2, 2023 16:07
@levadadenys levadadenys marked this pull request as draft February 2, 2023 16:07
@levadadenys levadadenys marked this pull request as ready for review February 2, 2023 19:07
@levadadenys
Copy link
Collaborator Author

@breville issues fixed

@@ -37,3 +37,5 @@ In Computer Science 101, the first program many students create is a simple one
{{ featured_project, title: "Animals", author: "B", age: "18+", image: "images/csc/helloworld/cschelloworld_animals.gif", alt_text: "A student project featuring an animated gif of an underwater scene with dolphin, fish, and jellyfish characters in front of a shipwreck", url: "https://studio.code.org/projects/spritelab/rYH8D8eAvWOjuiOpWbHzN4HAtis4ykKTIjGcNPP9zD4" }}
{{ featured_project, title: "Retro", author: "W", age: "13+", image: "images/csc/helloworld/cschelloworld_retro.gif", alt_text: "A student project featuring an animated gif of a game of tag with two alien characters on a distant planet", url: "https://studio.code.org/projects/spritelab/rYH8D8eAvWOjuiOpWbHzN7yo2E1S0q87VqlzaBz7oqo" }}
{{ clearfix }}

{{ helloworld }}
Copy link
Member

Choose a reason for hiding this comment

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

I think until this file gets re-translated (which can take a really long time, unless we deliberately flush the translations out) then a non-en user will see the old version of this file, albeit with new CSS around it. Will that look alright?

cc @code-dot-org/i18n

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@code-dot-org/i18n ping

Copy link
Member

Choose a reason for hiding this comment

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

I think the easiest path forward would be to find a way to update these without touching any of the .md.partial files...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure that's possible(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see 2 options here, but only one is workable( :

  1. Edit one of components/partials used in this file before, however it will affect all of the pages that render that component/partial
  2. (the one that's implemented right now) create new component/partial with all the specific html for that page

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And, while I was writing previous comment I've thought of making some global js logic that will inject html/style directly into the rendered page. However, I'm not sure if it's worth it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've also asked @kelbyhawn , but she had no ideas as well at that time.

However, the issue is indeed there. This way styles will affect only eng version of the site. If you know who we/I can ask about how to fix it, I'll be much obliged)

Screenshot from 2023-02-09 23-06-16
Screenshot from 2023-02-09 23-06-39

Copy link
Member

Choose a reason for hiding this comment

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

Okay, it looks like the best solution will be to add this new entry to the file, since (from offline discussions) the page will look alright (but older) without it, and once each language gets an updated "translation" of the page, it will get the newer look.

* update codeWorkspace

* update video modal

* update code work space/instructions, buttons

* update codeworkspace, instructions, etc

* Mark review fixes

* review fix

* review fix
…50233)

* update codeworksSpace, Js Debugger icons styling

* update codeWorkspace buttons styling on hover

* update modals and modals buttons

* review fix
…eral

# Conflicts:
#	apps/src/common-styles.module.scss
#	apps/src/templates/pane-header.module.scss
#	shared/css/phase1-design-system.scss
Copy link
Contributor

@kelbyhawn kelbyhawn left a comment

Choose a reason for hiding this comment

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

I think this PR looks good overall, but the text links on the /dance page in the screenshot below are getting overwritten by the old purple color — this link color will get updated universally in Phase II soon so I don't think it's blocking, but can you update this before merge if it works w/ your deadline?

Screenshot 2023-03-13 at 2 19 16 PM

Screenshot 2023-03-13 at 2 19 31 PM

…50703)

* share dialog

* advanced share rebrand

* share modal rebrand updates
@levadadenys
Copy link
Collaborator Author

levadadenys commented Mar 14, 2023

I think this PR looks good overall, but the text links on the /dance page in the screenshot below are getting overwritten by the old purple color — this link color will get updated universally in Phase II soon so I don't think it's blocking, but can you update this before merge if it works w/ your deadline?

Screenshot 2023-03-13 at 2 19 16 PM Screenshot 2023-03-13 at 2 19 31 PM

Thanks for noticing! Issue fixed!)

…rkspace changes (#50676)

* show code modal update

*  update VersionHistory.jsx modal

* update HintPrompt.jsx
@levadadenys levadadenys merged commit 908966e into staging Mar 15, 2023
@levadadenys levadadenys deleted the denys/phase1.5/general branch March 15, 2023 16:44
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.

None yet

3 participants