Appearance Changes #53

mattregul · 2018-07-31T20:38:49Z

Now uses weblegends style.css
Has own stylesheet (aistyle.css) for df-ai specific items (nav menu & stock report's table)
Chopped original nav menu up into chunks (likely over-engineered)
Added “Home” link, to return to weblegends homepage

- Now uses weblegends style.css - Has own stylesheet (aistyle.css) for df-ai specific items (nav menu & stock report's table) - Chopped original nav menu up into chunks (likely over-engineered) - Added “Home” link, to return to weblegends homepage

BenLubar

In addition to the individual comments, can you rebase (or merge) this on top of the master branch? 64e341c is required for the build to work on Linux.

BenLubar · 2018-08-01T10:44:13Z

weblegends.cpp

@@ -14,6 +14,16 @@ extern bool & enabled;

 REQUIRE_GLOBAL(world);

+enum AIPages {


Can you reformat this to use the DFHack coding style? Basically, { should go on the next line.

Also, changing this from enum to enum class will stop it from going into the global namespace. Right now, Plan (enum item) conflicts with Plan (class name).

BenLubar · 2018-08-01T10:45:03Z

weblegends.cpp

@@ -35,9 +45,124 @@ std::string html_escape(const std::string & str)
    return escaped;
 }

+std::string getAIPageURL(const int currentPage){


Instead of int, this should use AIPages.

Also, the { should go on the next line to match the DFHack coding style.

BenLubar · 2018-08-01T10:45:36Z

weblegends.cpp

+    }
+}
+
+int getAIPage(const std::string url){


Instead of int, this should use AIPages.

Also, the { should go on the next line to match the DFHack coding style.

BenLubar · 2018-08-01T10:46:09Z

weblegends.cpp

+        return AIPages::Status;
+}
+
+void create_nav_menu(weblegends_handler_v1 & handler, const std::string url)


const std::string url should be changed to const std::string & url to avoid copying the string.

BenLubar · 2018-08-01T10:47:10Z

weblegends.cpp

+    std::string title;
+    int currentPage = 0;
+    // Get current page
+    currentPage = getAIPage(url);


This should be AIPages instead of int, and you don't need to assign 0 to the variable before you call the function.

BenLubar · 2018-08-01T10:57:00Z

weblegends.cpp

+    //     return Report;
+    if (url == "/report/plan")
+        return AIPages::Report_Plan; // Tasks
+    else if (url == "/report/population")


Since every if in this function returns, you can remove all the elses.

BenLubar · 2018-08-01T11:01:35Z

weblegends.cpp

@@ -14,6 +14,16 @@ extern bool & enabled;

 REQUIRE_GLOBAL(world);

+enum AIPages {
+    Status,             // Status
+    //Report,           // Report - No longer used


We don't need the Report enum item, even in a comment. It just makes the code messier.

BenLubar · 2018-08-01T11:07:21Z

weblegends.cpp

+            title = "DF-AI - Status";
+    }
+    // Build Nav Menu =========
+    handler.cp437_out() << "<!DOCTYPE html><html lang=\"en\"><head><title>" << title << "</title><base href=\"../..\"/><link rel=\"stylesheet\" href=\"style.css\"/><link rel=\"stylesheet\" href=\"df-ai/aistyle.css\"/></head>";


<base href="../.."/> should only be used for pages with two slashes in their relative url. <base href=".."/> is for pages like df-ai/version and df-ai/plan, and there should not be a base tag at all for the df-ai status page.

BenLubar · 2018-08-01T11:08:05Z

weblegends.cpp

 bool ai_weblegends_handler(weblegends_handler_v1 & handler, const std::string & url)
 {
-    if (url == "/style.css")
+    // Requesting Sytlesheet


You spelled Stylesheet wrong here 😛

BenLubar · 2018-08-01T11:10:14Z

weblegends.cpp

    if (!enabled || !dwarfAI)
    {
        handler.status_code() = 503;
        handler.status_description() = "Service Unavailable";
-        handler.cp437_out() << "<!DOCTYPE html><html lang=\"en\"><head><title>df-ai</title></head>";
+        handler.cp437_out() << "<!DOCTYPE html><html lang=\"en\"><head><title>df-ai - Service Unavailable</title></head>";


Service Unavailable is probably too technical for most df-ai users (the previous line is part of the HTTP implementation.) Maybe something like df-ai - not active?

Also, this page does not include any stylesheets. The base tag might be a bit complicated to implement here.

- Removed commented items related to the old "Report" - switched AIPages to an enum class - getAIPageURL() now accepts AIPages, was int - getAIPage() now returns AIPages, was int - create_nav_menu() is always called now, ensuring styles are included in every page - - currentPage is now an AIPages instead of int - - simplified how each of the menu links are built, removing repetitive code - Reworked displaying the "AI Not active" message when df-ai isn't running - - Page title is set in create_nav_menu() - - Added a help message and link to wiki to aid user - - If df-ai isn't running, clicking on any menu item displays the "not active screen and a help message - Base url changes depending on your navigation level (status and the "not active" page have a base, so the "Home" menu item returns user to the weblegends menu - fixed typos - Added new CSS class "navItem" to each menu item, applying a 12px right padding - removed " - " between each menu item

mattregul · 2018-08-01T15:29:39Z

I think I did the rebase right?

Appearance Changes - pt2

Removed commented items related to the old "Report"
switched AIPages to an enum class
getAIPageURL() now accepts AIPages, was int
getAIPage() now returns AIPages, was int
create_nav_menu() is always called now, ensuring styles are included in every page
- currentPage is now an AIPages instead of int
- simplified how each of the menu links are built, removing repetitive code
Reworked displaying the "AI Not active" message when df-ai isn't running
- Page title is set in create_nav_menu()
- Added a help message and link to wiki to aid user
- If df-ai isn't running, clicking on any menu item displays the "not active screen and a help message
Base url changes depending on your navigation level (status and the "not active" page have a base, so the "Home" menu item returns user to the weblegends menu
fixed typos
Added new CSS class "navItem" to each menu item, applying a 12px right padding
removed " - " between each menu item

BenLubar · 2018-08-01T15:34:29Z

weblegends.cpp

            title = "DF-AI - Status";
+            baseURL = "/";


This should be ./ or just an empty string. Otherwise, it's like an infinite number of ../../...

BenLubar · 2018-08-01T15:35:32Z

weblegends.cpp

-        handler.status_description() = "Service Unavailable";
-        handler.cp437_out() << "<!DOCTYPE html><html lang=\"en\"><head><title>df-ai - Service Unavailable</title></head>";
-        handler.cp437_out() << "<body><p><i>AI is not active.</i></p></body></html>";
+        handler.status_description() = "df-ai - Not Active";


That should be Service Unavailable. status_description isn't shown to people, it's for the benefit of machines.

- Fixed base for status and "not active" screen - Fixed "not active" description to "Service Unavailable"

Appearance Changes

cb80e3e

- Now uses weblegends style.css - Has own stylesheet (aistyle.css) for df-ai specific items (nav menu & stock report's table) - Chopped original nav menu up into chunks (likely over-engineered) - Added “Home” link, to return to weblegends homepage

BenLubar requested changes Aug 1, 2018

View reviewed changes

mattregul added 2 commits August 1, 2018 11:14

Merge remote-tracking branch 'upstream/master'

0299715

Removed extra blank line

e442d15

BenLubar reviewed Aug 1, 2018

View reviewed changes

Base and desc fix

8397a09

- Fixed base for status and "not active" screen - Fixed "not active" description to "Service Unavailable"

BenLubar approved these changes Aug 1, 2018

View reviewed changes

BenLubar merged commit f8b297b into BenLubar:master Aug 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Appearance Changes #53

Appearance Changes #53

mattregul commented Jul 31, 2018

BenLubar left a comment

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

mattregul commented Aug 1, 2018 •

edited

Loading

BenLubar Aug 1, 2018

BenLubar Aug 1, 2018

		@@ -14,6 +14,16 @@ extern bool & enabled;

		REQUIRE_GLOBAL(world);

		enum AIPages {

Appearance Changes #53

Appearance Changes #53

Conversation

mattregul commented Jul 31, 2018

BenLubar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattregul commented Aug 1, 2018 • edited Loading

Appearance Changes - pt2

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattregul commented Aug 1, 2018 •

edited

Loading