Skip to content

sheet.php: GAME_TITLE and navigation fixes - #13

Closed
juliettef wants to merge 4 commits into
ramiismail:masterfrom
juliettef:master
Closed

sheet.php: GAME_TITLE and navigation fixes#13
juliettef wants to merge 4 commits into
ramiismail:masterfrom
juliettef:master

Conversation

@juliettef

Copy link
Copy Markdown

Fixes to the projects pages (sheet.php) correct the following:

  • Removed the 'projects' navigation item as this does not exist on the project page, and added 'features' navigation item and id as these were missing.
  • Set the game title and navigation title to GAME_TITLE
  • Add an id="game-title" to the navigation title so that this can be picked up by the email script validation.js, which fixes a bug where emails had 'undefined' in the email subject.

…name, and game-title id set for request email use.
…e home page. Improved feedback on invalid email address entry, error sending message and success.
@juliettef

Copy link
Copy Markdown
Author

We've (with @dougbinks) also fixed the mail validation script for the standard installation. Also added some error handling and user feedback.

@qwiboo

qwiboo commented Mar 6, 2014

Copy link
Copy Markdown

These are great fixes, please pull them asap.

@juliettef

Copy link
Copy Markdown
Author

It looks like @Dearon has made a change (7416a50) which fixes one of the issues we fixed (mail) after we made this pull request. So this pull request won't merge without conflicts.

We could do the work needed to integrate @daeron's change with our other fixes but only if there's some chance that @ramiismail thinks this is worthwhile doing (this would also allow us to submit a merge request for the google app engine integration).

Alternatively @ramiismail can cherry pick the two game page fixes.

@Dearon

Dearon commented Mar 6, 2014

Copy link
Copy Markdown
Contributor

@juliettef Yeah, I fixed them after askin Rami if there is something specific he wanted to see fixed. Your best bet to get things merged is to keep poking him on a regular basis, it's not ideal but it has worked for me so far :)

@juliettef

Copy link
Copy Markdown
Author

Closing this pull request due to conflict (mail).
Moved navigation fixes alone to new pull request: #20

@juliettef juliettef closed this Mar 9, 2014
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