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

Tag Settings Menu Validation #5865

Merged
merged 1 commit into from Oct 14, 2015

Conversation

Projects
None yet
2 participants
@acburdine
Member

acburdine commented Sep 23, 2015

refs #5845

Todos:

  • Add validation to tag description
  • Add char counter to description field
  • Fix tag description css

@acburdine acburdine changed the title from [WIP] Tag Settings Menu Validation to Tag Settings Menu Validation Oct 9, 2015

add tag description validation to tag settings menu
refs #5845

- adds validation to description field
- fixes css to word-wrap on words that are too long
@kevinansfield

This comment has been minimized.

Show comment
Hide comment
@kevinansfield

kevinansfield Oct 13, 2015

Contributor

👍 looks good so far, thanks @acburdine! Would you also be able to add some validation to the slug? Right now it lets you enter a slug that's long enough (> 150 chars) to fail on the server side.

There's also a problem when we do receive an error from the server, I think this line (https://github.com/TryGhost/Ghost/blob/master/core/client/app/controllers/settings/tags.js#L53) needs to be changed to self.get('notifications').showAPIError.

Contributor

kevinansfield commented Oct 13, 2015

👍 looks good so far, thanks @acburdine! Would you also be able to add some validation to the slug? Right now it lets you enter a slug that's long enough (> 150 chars) to fail on the server side.

There's also a problem when we do receive an error from the server, I think this line (https://github.com/TryGhost/Ghost/blob/master/core/client/app/controllers/settings/tags.js#L53) needs to be changed to self.get('notifications').showAPIError.

@kevinansfield

This comment has been minimized.

Show comment
Hide comment
@kevinansfield

kevinansfield Oct 14, 2015

Contributor

@acburdine I'm about to start a fair bit of work in this area so I'm going to merge this as-is for now. Thanks again 😄

Contributor

kevinansfield commented Oct 14, 2015

@acburdine I'm about to start a fair bit of work in this area so I'm going to merge this as-is for now. Thanks again 😄

kevinansfield added a commit that referenced this pull request Oct 14, 2015

@kevinansfield kevinansfield merged commit 8af13e9 into TryGhost:master Oct 14, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ErisDS ErisDS referenced this pull request Oct 14, 2015

Closed

Tag Management UI Updates #5845

10 of 10 tasks complete

@acburdine acburdine deleted the acburdine:tag-menu-charcount branch Oct 17, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment