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

Dec2020 small updates #140

Merged
merged 8 commits into from
Jan 6, 2021
Merged

Dec2020 small updates #140

merged 8 commits into from
Jan 6, 2021

Conversation

vash3g
Copy link
Member

@vash3g vash3g commented Jan 2, 2021

Updates to: main page graph for clarity, spacer in chapter view, tournament info information, tournament quals information.

Updated information page to display new TDList information in full.
Added tournament ratings steps
Add space between chapter info and members
@vash3g vash3g self-assigned this Jan 2, 2021
@@ -46,7 +46,7 @@ <h2> Chapter: {{ chapter.code }} </h2>
<dt> Website: </dt> <dd> <a href="{{chapter.url}}">{{ chapter.url }}</a> </dd>
{% endif %}
</dl>

<p></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you do something different here for your formatting? I am not sure we should be using <p> and </p> tags here if we are looking for a line break. CSS or Inline style maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

I could add a double break line
in its place. Wanted just a large spacing there.

agagd/agagd_core/tables.py Outdated Show resolved Hide resolved
.style("text-anchor", "end")
.text("total # of Games");
.style("text-anchor", "start")
.text("Total # of Games");
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at this a second time. I don't think we need the "Total # of Games" or "# of Games" on both sides. I think we could do with just the one on the left with your new label "Total # of Games" vs "# of Games."

Copy link
Member Author

Choose a reason for hiding this comment

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

One has been hidden so people dont like the graph. Trying to fix the graph to correctly show what it is until we can overhaul it with other info as per #122 .

Copy link
Contributor

Choose a reason for hiding this comment

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

My issue is that we are introducing a new issue here, so now we have the label on both sides. One of which may be obscured by our graph in production.
image

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I was playing with it on the test server and then replicating it for my git files tbh. The right side test should still sit below the graph lines.

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked the test server, and I am seeing an error 500, so I have reverted that. If you would like to mess with this locally, try doing so with $ docker-compose up --build --force recreate. You should be able to download the schema from https://github.com/usgo/usgo-sql-schemas. There are instructions for basic docker-compose operations on: https://github.com/usgo/agagd/blob/master/docs/docker.md. The only two files that are really needed to run $ docker-compose up are: schema.sql and config-docker.env. It may be finicky on the values for config-docker.env. Let me know if you have any issues.

Otherwise, the change does make sense.

Comment on lines 29 to 47
<li>Tournament Directors: <br>
<table>
<tr>
<th>TD List Name</th>
<th><a href="https://usgo.org/mm/tdlista.txt">TDListA</a></th>
<th><a href="https://usgo.org/mm/tdlistb.txt">TDListB</a></th>
<th><a href="https://usgo.org/mm/tdlistn.txt">TDListN</a></th>
</tr><tr>
<th>Delimiter</th>
<th>Tab</th>
<th>Tab</th>
<th>Tab</th>
</tr><tr>
<th>Columns displayed</th>
<th>MememberName AGAID MememberType Rating RenewalDue ChapterCode State Sigma RatingUpdated</th>
<th>MememberName AGAID MememberType Rating RenewalDue ChapterName State Sigma RatingUpdated</th>
<th>MememberName AGAID MememberType Rating RenewalDue ChapterCode State || Fixed Width</th>
</tr>
</table> </li>
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, I think these should be a little clearer that we are using tab delimitation for our TDLists.

I think we could do something about the presentation of the table and it should be a different section. Under the bullet list may not be clear to some.
image

Copy link
Member Author

Choose a reason for hiding this comment

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

Youre right, I was writing these one way and they need to be the other way. I will resubmit this.

Updated to better show information
@@ -26,7 +26,25 @@ <h2>Additional Information</h2>
<li>Answers to the <a href="https://www.usgo.org/files/pdf/ratingquestions.pdf">most commonly asked questions about the rating system</a> (May 1992)</li>
<li>A more detailed <a href="https://www.usgo.org/files/pdf/AGARatings-Math.pdf">mathematical description</a> of the ratings system (April 2010)</li>
<li><a href="https://www.usgo.org/files/pdf/bayrate.zip">Source code</a> for the AGA rating algorithm</li>
<li>Tournament Directors: <a href="https://usgo.org/mm/tdlista.txt">TDListA</a> <a href="https://usgo.org/mm/tdlistn.txt">TDListN</a></li>
<li>Tournament Directors: <br>
Copy link
Contributor

Choose a reason for hiding this comment

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

We would probably do better moving the title into an <h3></h3> or another header; then moving the table beneath that and having the label "TDLists Formating." May be change the label for "Additional information" to "Resources."

Copy link
Member Author

Choose a reason for hiding this comment

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

The page probably needs an overhaul of info anyways. Next update?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. We could just move this forward, and on the next update the page can be overhauled.

@@ -46,7 +46,7 @@ <h2> Chapter: {{ chapter.code }} </h2>
<dt> Website: </dt> <dd> <a href="{{chapter.url}}">{{ chapter.url }}</a> </dd>
{% endif %}
</dl>

<br><br>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mind having two breaks. I just think that one may may the new version softer compared with the present change. The new and old screenshots are just references to #140's new change and our current version.

New
image

Old
image

Copy link
Member Author

Choose a reason for hiding this comment

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

One Two, As long as theres a slight break between the two I think it looks better to break up the sections. Unlike the odd way that last sentence is structured.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have updated my comment: #140 (comment) to make more sense.

@michaelhiiva michaelhiiva merged commit 327f457 into usgo:master Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants