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

Allow trailing space in card entry #39

Closed
wants to merge 1 commit into from

Conversation

Nightfirecat
Copy link
Contributor

Fixes #38

@tooomm
Copy link

tooomm commented Mar 31, 2018

Will this "fix" leading spaces as well?
I can easily imagine that people run into the same problem there.

@Nightfirecat
Copy link
Contributor Author

Leading spaces as in the following? No; that would require a regex change. That said, I don't think it would be too hard to implement.

      1 Tarmogoyf

Nightfirecat added a commit to Nightfirecat/decklist that referenced this pull request Apr 2, 2018
@Nightfirecat
Copy link
Contributor Author

@tooomm Fyi, opened #40 which would address your comment.

Nightfirecat added a commit to Nightfirecat/decklist that referenced this pull request Apr 10, 2018
In addition to adding the StarCityGames decksheet format, a number of
under-the-covers changes occurred to facilitate generalizing the
codebase to allow multiple decksheet layouts. Some of these changes
are as follows:

- Decklist parsing (decklist.js) has been fully re-written. All new
  functions accept parameters (usually optional to resemble previous
  behavior as much as possible) and utilize return values rather than
  modifying global variables to allow more predictable behavior, better
  compatibility between decksheets (individual decksheets can utilize
  the return value as needed), and improve ease of testing. The primary
  reason for rewriting parsing was to utilize full card objects (like
  those stored in the global `cards` array) in decklist layout rather
  than simple name-quantity arrays.
- All extraeneous global variables have been removed. The only two that
  remain are necessary for the validation and PDF update timers to
  function.
- The page layout has been updated to include a second radio selection
  above the decklist preview with options for layouts. Additionally,
  both radio selection lists have had their `name` attributes updated
  for ease of selection.
- Both SCG and WotC PDF layout functions have been re-ordered such that
  copy-pasting text from PDFs displays in a more sane format
- `eslint` directives have been added to main.js and decklist.js to
  minimize errors
- Extra js files have been removed
- (incidental) A font sizing bug in the WotC decklist layout has been
  fixed. (previously if no last name was set, font size was too small)
- (incidental) Centered text is now actually centered in the WotC
  decklist layout

With all that said, there are some other possible features that have not
been included in this PR, such as:

- Section overflow looping. Currently, the SCG decklist caps each
  section at 15 cards, and does not push overflow into further (or
  previous) sections.
- Adding handling of a new GET parameter to select decklist. As there
  was no handling for deck sorting, similarly support was not added for
  decklist choice in this PR.
- Disabling of unused fields in specific layouts. The SCG decksheet
  layout does not use all of the same fields which are used in the WotC
  format. (specifically, it does not make use of the Event Date, Event
  Location, and Deck Designer fields) Current behavior is to simply
  ignore the value of these fields while parsing, but allow further
  changes to the fields.
- Smooth out added images. For some reason, they show up rather jagged,
  despite being downscaled from their source images. This may be related
  to parallax/jsPDF#762
  Closest example?: parallax/jsPDF#762 (comment)
- Utilize the full-resolution StarCityGames.com logo. Currently, for
  load time consideration, a low-resolution version of the
  StarCityGames.com logo is loaded, though a higher-resolution copy is
  available in the repository. Ideally, this image and others should no
  longer be loaded as base64 strings, but loaded via FileReader or some
  other API.
  Ref: https://stackoverflow.com/a/20285053

Finally, due to the update to parsing, there are some changes to
existing behavior--primarily small changes in behavior of how
unrecognized cards are sorted in lists. As much as possible, the new
parsing code behaves similarly to existing code, with the caveat that
sorting is not necessarily stable, which likely explains the
previously-mentioned sorting discrepancies.

Closes april#8
Fixes april#38
Supersedes april#35
Supersedes april#39
Supersedes april#40
Nightfirecat added a commit to Nightfirecat/decklist that referenced this pull request Apr 10, 2018
In addition to adding the StarCityGames decksheet format, a number of
under-the-covers changes occurred to facilitate generalizing the
codebase to allow multiple decksheet layouts. Some of these changes
are as follows:

- Decklist parsing (decklist.js) has been fully re-written. All new
  functions accept parameters (usually optional to resemble previous
  behavior as much as possible) and utilize return values rather than
  modifying global variables to allow more predictable behavior, better
  compatibility between decksheets (individual decksheets can utilize
  the return value as needed), and improve ease of testing. The primary
  reason for rewriting parsing was to utilize full card objects (like
  those stored in the global `cards` array) in decklist layout rather
  than simple name-quantity arrays.
- All extraeneous global variables have been removed. The only two that
  remain are necessary for the validation and PDF update timers to
  function.
- The page layout has been updated to include a second radio selection
  above the decklist preview with options for layouts. Additionally,
  both radio selection lists have had their `name` attributes updated
  for ease of selection.
- Both SCG and WotC PDF layout functions have been re-ordered such that
  copy-pasting text from PDFs displays in a more sane format
- `eslint` directives have been added to main.js and decklist.js to
  minimize errors
- Extra js files have been removed
- (incidental) A font sizing bug in the WotC decklist layout has been
  fixed. (previously if no last name was set, font size was too small)
- (incidental) Centered text is now actually centered in the WotC
  decklist layout

With all that said, there are some other possible features that have not
been included in this PR, such as:

- Section overflow looping. Currently, the SCG decklist caps each
  section at 15 cards, and does not push overflow into further (or
  previous) sections.
- Adding handling of a new GET parameter to select decklist. As there
  was no handling for deck sorting, similarly support was not added for
  decklist choice in this PR.
- Disabling of unused fields in specific layouts. The SCG decksheet
  layout does not use all of the same fields which are used in the WotC
  format. (specifically, it does not make use of the Event Date, Event
  Location, and Deck Designer fields) Current behavior is to simply
  ignore the value of these fields while parsing, but allow further
  changes to the fields.
- Smooth out added images. For some reason, they show up rather jagged,
  despite being downscaled from their source images. This may be related
  to parallax/jsPDF#762
  Closest example?: parallax/jsPDF#762 (comment)
- Utilize the full-resolution StarCityGames.com logo. Currently, for
  load time consideration, a low-resolution version of the
  StarCityGames.com logo is loaded, though a higher-resolution copy is
  available in the repository. Ideally, this image and others should no
  longer be loaded as base64 strings, but loaded via FileReader or some
  other API.
  Ref: https://stackoverflow.com/a/20285053

Finally, due to the update to parsing, there are some changes to
existing behavior--primarily small changes in behavior of how
unrecognized cards are sorted in lists. As much as possible, the new
parsing code behaves similarly to existing code, with the caveat that
sorting is not necessarily stable, which likely explains the
previously-mentioned sorting discrepancies.

Closes april#8
Fixes april#32
Fixes april#38
Supersedes april#35
Supersedes april#39
Supersedes april#40
@Nightfirecat
Copy link
Contributor Author

Implemented in #42.

@Nightfirecat Nightfirecat deleted the allow-trailing-space branch June 6, 2018 14:50
This pull request was closed.
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.

2 participants