General Changes #15

lejshi3 · 2020-11-18T14:47:49Z

I replaced the black squares on the home page with semi-related placeholders using images from the CC tutorial screens. These changes are placeholders and not meant to be final, they're just supposed to make putting in new images in their place easier in the future, also they look nicer than black squares.

Note: If this Pull Request is Accepted, Please Change the "Homepage": "https://sirjoshi.github.io/Cortex-Command-Community.github.io/", with "Homepage": "Cortex-Command-Community.github.io" or delete the line altogether or else it will show a blank webpage when deployed.

facebook robot code bad, indented human code good.

I replaced the black squares with pictures from the game's tutorial files. I would not call these changes final, but it will save some time for when someone has new pictures/gifs to go here. the quest for the pink role continues

…ommunity.github.io

i am in pain, i am going to make a pull request soon

maybe this will work?

added alt text

garethyr

I'll leave code structure comments to Tom, here's some cleanup for you to do while you wait for his review.

garethyr · 2020-11-18T16:41:24Z

package.json

@@ -2,6 +2,7 @@
  "name": "cortex",
  "version": "0.1.0",
  "private": true,
+  "homepage": "https://sirjoshi.github.io/Cortex-Command-Community.github.io/",


It's not a big deal, but you should do a little cleanup - in your dev environment, change this yourself and commit it to your relevant branch so it doesn't show up as cruft here.

garethyr · 2020-11-18T16:41:36Z

public/index.html

@@ -28,7 +28,7 @@
    <title>Cortex Command Community Project</title>
  </head>
  <body>
-    <noscript>You need to enable JavaScript to run this app.</noscript>
+    <noscript>You need to enable JavaScript to view this page, nerd.</noscript>


Undo this please and thanks.

garethyr · 2020-11-18T16:43:05Z

src/components/Home/Home.js

@@ -7,6 +7,12 @@ import CortexCardContainer from './CortexCardContainer/CortexCardContainer';
 import CortexCardContents from './CortexCardContents/CortexCardContents';
 import MagicArrow from './MagicArrow/MagicArrow';

+/*i pray to god (if there is one) that there is a beter way to go about this than just manually importing every images*/


Kill off this comment please.
For your future knowledge, a good way to do this sort of thing is leave your own comment on the PR, explaining that you're unsure of whether this is the right way to do something. It means cleaner code and it's pretty much impossible for reviewers to miss.

garethyr · 2020-11-18T16:44:47Z

src/components/Home/Home.js

@@ -27,19 +33,24 @@ function Home() {
                        <CortexCardContents
                            title='Upgraded Engine'
                            body='The source code has been enhanced to improve performance.'
+                            image={DummySquadImg}
+                            alt='some dummies'


Please change the alt text slightly so it's a little better/more uniform:
some dummies -> Dummy Squad Image
a buy menu -> Buy Menu Image
a joystick -> Joystick Image

made much more descriptive alt texts

professionalism

deleted the homepage property

lejshi3 · 2020-11-18T17:07:34Z

Some changes have been made in the name of Accessibility and Professionalism

not important, just was bugging me

1. Modified the Page to be Dark not Light (Light is gross) 2. Made the 404 Page Cooler 3. Made the Downloads have a direct link to Pre2 and a Get Mods section that links to the Discord and the Mod.io 4. Modified the Downloads page to make the border go even further arround the text (it just looks cooler)

lejshi3

Changes:

I changed a lot of CSS and Style to make the Pages Dark as Opposed to Light
I added placeholder images to the home page to make adding in new pics/gifs easier
I no longer call js-phobics nerds
I made some subtle changes to the style of the Downloads page
I added a direct link to Pre 2 in the Downloads page
I added a Get Mods section to the Downloads page that links to the Discord and the Mod.io

tomflenniken

Looking good, I definitely like the new placeholder images. I agree gifs would be nice, but this is a great start.

tomflenniken · 2020-11-19T00:07:55Z

src/components/Home/CortexCardContents/CortexCardContents.js

                </div>
            </div>
            {/* This is an image placeholder **Imagine an Image** */}
-            <div style={{
+        <img style={{


I believe the original indentation was correct here.

tomflenniken · 2020-11-19T00:13:38Z

src/components/Home/Home.js

                        />
                    </CortexCard>
                    <CortexCard>
                        <CortexCardContents
                            title='Improved Strategic Gameplay'
                            body='The team has focused on accentuating the most satisfying gameplay elements.'
+                            image={JoystickImg}
+                            alt='A Handdrawn/Pixelart Image of a Joystick, an input method that can be used to (ineffiecently) play Cortex Command.'


probably too long for alt text - though I appreciate the levity.
Maybe cut it to A pixel art image of a joystick.

Also I think sentence case is appropriate for these alt texts.
I would also avoid using "/" because a primary use for alt text is screen readers, and I don't think they'd read that out in a natural sounding way.

Thanks for providing alt text, it's probably not super critical for a site for a game that has 0 accessibility features, but it helps for making the web a bit more inclusive.

tomflenniken · 2020-11-19T00:15:26Z

src/components/Static/NotFound/NotFound.js

            <h1 style={{ marginTop: '48px' }}>Page Not Found</h1>
            <p>Sorry, but the page you were trying to view does not exist.</p>
+            <br />
+            <br />


I'm not wholly opposed to using <br /> here (I might have used it some places myself), but generally I think using CSS for spacing should be preferred.

Don't change it if you don't feel like it.

tomflenniken · 2020-11-19T00:17:43Z

src/components/Static/NotFound/NotFound.js

            <h1 style={{ marginTop: '48px' }}>Page Not Found</h1>
            <p>Sorry, but the page you were trying to view does not exist.</p>
+            <br />
+            <br />
+            <a href='https://www.youtube.com/watch?v=t0s9dRNjhfg?autoplay=1'><img style={{postion: 'absolute', width: '25%'}} src={EmojiDunno}/></a>


Interesting easter egg. Doesn't seem super relevant in this context, but I'm fine with it.

tomflenniken · 2020-11-19T00:28:28Z

src/components/App/App.css

@@ -1,6 +1,6 @@
 body {
-  color: #111;
-  background-color: #eee
+  color: #EEE;


I like the light theme :(

Actually, can you pull out the theme edits into a separate branch? I think it needs to be addressed separately. I think it's a good opportunity to adjust the stylesheets and how style is managed across the site.
See #9

tomflenniken · 2020-11-19T00:52:25Z

src/components/Static/NotFound/NotFound.js

 function NotFound() {
    return (
        <Template>
+            <div style={{textAlign: 'center'}}>


I think left aligned is more professional looking, at least for text.

tomflenniken · 2020-11-19T00:54:36Z

src/components/Template/Template.js

@@ -12,8 +12,10 @@ function Template(props) {
      <Header />
      <div style={{
        margin: 'auto',
+        marginBottom: '0px',


What are the changes in this file for?

tomflenniken · 2020-11-19T00:56:50Z

src/components/Home/Home.js

@@ -7,6 +7,11 @@ import CortexCardContainer from './CortexCardContainer/CortexCardContainer';
 import CortexCardContents from './CortexCardContents/CortexCardContents';
 import MagicArrow from './MagicArrow/MagicArrow';

+/*image imports*/


I think prettier or something enforces import ordering, so I typically just put all the imports together so the autoformatter can do its thing.

tomflenniken · 2020-11-19T01:00:23Z

src/components/Static/Downloads/Downloads.js

+        <Button link to='https://github.com/cortex-command-community/Cortex-Command-Community-Project-Data/releases/download/v0.1.0-pre2/CCCP.v0.1.0-pre2.zip'>Pre 2 <img width='14px' height='14px' marginBottom='-12px' src={EmojiGood}/> (latest)</Button>
+
+      </Card>
+      <h2>Mods</h2>


With the addition of some more content on this page I think it's looking a little cluttered. No action required now, but I will probably be restructuring some of the headings on this page/grouping up some of the card content.

tomflenniken · 2020-11-19T01:02:05Z

src/components/Static/Downloads/Downloads.js

        <Button link to='https://github.com/cortex-command-community/Cortex-Command-Community-Project-Data/releases'>Releases</Button>
+
+        <Button link to='https://github.com/cortex-command-community/Cortex-Command-Community-Project-Data/releases/download/v0.1.0-pre2/CCCP.v0.1.0-pre2.zip'>Pre 2 <img width='14px' height='14px' marginBottom='-12px' src={EmojiGood}/> (latest)</Button>


marginBottom here is giving a console error. It should be defined as part of a style attribute
style={{ marginBottom= ... }}

tomflenniken · 2020-11-19T01:06:36Z

I noticed a responsiveness issue for the buttons that should be addressed.

A New Picture and a New Button

…unity.github.io

lejshi3 · 2021-02-06T17:16:19Z

Okay, Hello, Happy 2021.

A lot of the issues above have been addressed so i'll keep this brief.

Added GIFS
Added Get Involved
Expanded on the Downloads Page

After the PR, I'd consider the website in a usable state.

tomflenniken · 2021-02-10T07:11:57Z

@sirjoshi can you split the changes out into separate branches for the different pages that they impact (or any other reasonable way to break down the changes here)?

I think we should tackle these in a more targeted manner, I think it will be easier to get your changes through in chunks.

lejshi3 added 14 commits October 20, 2020 17:06

beautified the index.html

91d72ab

facebook robot code bad, indented human code good.

Replaced the Placeholders with Placeholders

6dab1e1

I replaced the black squares with pictures from the game's tutorial files. I would not call these changes final, but it will save some time for when someone has new pictures/gifs to go here. the quest for the pink role continues

Merge branch 'master' of https://github.com/SirJoshi/Cortex-Command-C…

0210564

…ommunity.github.io

Update index.html

ce35043

Update index.html

e1f907e

mmmmmm

62d6d35

i am in pain, i am going to make a pull request soon

jhmmmm

e45ae9f

maybe this will work?

idk anymore

c1f7ca0

Update package.json

cd4f8d8

Update index.html

deb48e2

please let this let it build on gh

c1982e8

added alt text

Update package.json

b2ef7a9

Update package.json

d2f734b

Update package.json

af8e01a

garethyr requested a review from tomflenniken November 18, 2020 16:39

garethyr suggested changes Nov 18, 2020

View reviewed changes

lejshi3 added 3 commits November 18, 2020 11:02

Update Home.js

db6c0f8

made much more descriptive alt texts

Update index.html

3c90b99

professionalism

Update package.json

403d334

deleted the homepage property

lejshi3 added 3 commits November 18, 2020 11:09

Update Home.js

c21d328

indentation

3f101e5

not important, just was bugging me

lejshi3 commented Nov 18, 2020

View reviewed changes

tomflenniken reviewed Nov 19, 2020

View reviewed changes

lejshi3 added 4 commits December 20, 2020 04:37

Misc Changes

7a54356

A New Picture and a New Button

get involved page

dd060e9

made it more cortexy

c35cb99

Merge https://github.com/cortex-command-community/Cortex-Command-Comm…

762ee5b

…unity.github.io

lejshi3 added 3 commits February 6, 2021 08:34

download page

d9a00e7

stuff and things

a155b07

changed the images into gifs

ff4450e

lejshi3 changed the title ~~Placeholderier Placeholders~~ General Changes Feb 6, 2021

lejshi3 requested a review from garethyr February 6, 2021 19:29

Update package.json

5415f1c

		<Button link to='https://github.com/cortex-command-community/Cortex-Command-Community-Project-Data/releases'>Releases</Button>

		<Button link to='https://github.com/cortex-command-community/Cortex-Command-Community-Project-Data/releases/download/v0.1.0-pre2/CCCP.v0.1.0-pre2.zip'>Pre 2 <img width='14px' height='14px' marginBottom='-12px' src={EmojiGood}/> (latest)</Button>

General Changes #15

Are you sure you want to change the base?

General Changes #15

Uh oh!

Conversation

lejshi3 commented Nov 18, 2020

Uh oh!

garethyr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lejshi3 commented Nov 18, 2020

Uh oh!

lejshi3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Changes:

Uh oh!

tomflenniken left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomflenniken commented Nov 19, 2020

Uh oh!

lejshi3 commented Feb 6, 2021

Uh oh!

tomflenniken commented Feb 10, 2021

Uh oh!

Uh oh!

lejshi3 left a comment •

edited

Loading