Skip to content

#3228 - add confirm modal for delete#3662

Merged
jpadilla merged 1 commit intoencode:masterfrom
awwester:ticket_3228
Dec 1, 2015
Merged

#3228 - add confirm modal for delete#3662
jpadilla merged 1 commit intoencode:masterfrom
awwester:ticket_3228

Conversation

@awwester
Copy link
Copy Markdown
Contributor

Added a modal to confirm deletion of data

@jpadilla
Copy link
Copy Markdown
Contributor

@awwester this is great! Mind putting together a screenshot or gif? Might also be a good idea to wrap new strings in a {% trans %} template tag`.

@jpadilla jpadilla added this to the 3.3.2 Release milestone Nov 22, 2015
@awwester
Copy link
Copy Markdown
Contributor Author

drf_confirm_delete

This is my first time working with translations and although the code seems correct comparing with other instances, I'm having issues getting a positive test to be sure. I've added a couple settings according to http://www.django-rest-framework.org/topics/internationalization/#enabling-internationalized-apis but can't get my app to switch into spanish for testing.

  # settings.py
  MIDDLEWARE_CLASSES = (
    'django.middleware.locale.LocaleMiddleware',
    ...
  )
  LANGUAGE_CODE = 'es-es'

is there something I'm missing?

@jpadilla
Copy link
Copy Markdown
Contributor

jpadilla commented Dec 1, 2015

@awwester sorry for the delay. To move this forward faster, I'd say forget about adding translation tags, drop the django.po. After that this is good to merge.

@awwester
Copy link
Copy Markdown
Contributor Author

awwester commented Dec 1, 2015

@jpadilla The trans changes have been reverted

@jpadilla
Copy link
Copy Markdown
Contributor

jpadilla commented Dec 1, 2015

@tomchristie @xordoquy any feedback? If it looks good, I'm merging.

@lovelydinosaur
Copy link
Copy Markdown
Contributor

No objection from me.

jpadilla added a commit that referenced this pull request Dec 1, 2015
@jpadilla jpadilla merged commit d2f90fd into encode:master Dec 1, 2015
@jpadilla
Copy link
Copy Markdown
Contributor

jpadilla commented Dec 1, 2015

@awwester thanks again 🎆

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.

3 participants