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

After saving show a modal containing edited mappings #193

Merged
merged 12 commits into from Feb 25, 2014
Merged

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Feb 24, 2014

  • After bulk editing, bulk tagging, single mapping edit and bulk adding always return the user to where they were and show a modal window containing their edited selection along with bulk edit tools for making quick fixes
  • For non-javascript users the existing flash message remains
  • For javascript users a modal shows which gives a confirmation message
    • The modal is dismissed by pressing the "Ok" button
    • Clicking the background on this modal does not close it, preventing accidental dismissal
  • Within the modal a table of just-edited mappings are shown, to allow users to check their changes
    • These mappings can be edited again using the bulk-editing technique with checkboxes and buttons

screen shot 2014-02-24 at 16 13 53

Paul Hayes added 11 commits Feb 20, 2014
* Caveat: For now itt redirects to the first page of mappings at the
moment, rather than the page you were on before you began editing.
* Preparation before using the table in a modal window
* Use flash messaging to pass through mapping IDs we can later re-use
* Fix the tag feature which broke in the process (multiple mappings
tables)
  * Create a way of distinguishing between the two and of addressing
exact mappings
* Hide bulk add and footer when showing a modal
* Return to the context that you left when editing a single mapping
* Use the js/non-js style helpers to hide the usual saved success
messages for users that can see modals
Paul Hayes
* Move inline JS out of modal include
* Update event listener to use new namespaced Bootstrap 3 event, rather
than the old Bootstrap 2 one
* In modal windows with short paths they were growing longer, and the
table looked incorrect
@jamiecobbett
Copy link
Contributor

@jamiecobbett jamiecobbett commented Feb 25, 2014

Presentation niggle: when the title of the modal is long, the "OK" button gets pushed down:
screen shot 2014-02-25 at 12 52 59

@@ -26,6 +26,7 @@ def create_multiple
bulk_add.create_or_update!

flash[:success] = bulk_add.success_message
flash[:saved_mapping_ids] = bulk_add.modified_mappings.map {|m| m.id}

This comment has been minimized.

@jamiecobbett

jamiecobbett Feb 25, 2014
Contributor

This stores the IDs in the session and our session is currently stored in a cookie. Cookies have a maximum size of 4kilobytes so I am able trigger a 502 error from nginx by manipulating more than about 60 mappings. I think we either need to store the session in the database or move these IDs to a querystring parameter. Would you be happy to try the latter?

This comment has been minimized.

@fofr

fofr Feb 25, 2014
Author Contributor

Won't a query string also be too long, and exceed the maximum URL length?

This comment has been minimized.

@jamiecobbett

jamiecobbett Feb 25, 2014
Contributor

Aha, yes. Damn.

This comment has been minimized.

@fofr

fofr Feb 25, 2014
Author Contributor

* Move styles for modal into a separate file
* Remove an ugly inline style
@jamiecobbett
Copy link
Contributor

@jamiecobbett jamiecobbett commented Feb 25, 2014

This is gorgeous 🐐 :goberserk:

jamiecobbett added a commit that referenced this pull request Feb 25, 2014
After saving show a modal containing edited mappings
@jamiecobbett jamiecobbett merged commit 2a43d4e into master Feb 25, 2014
1 check passed
1 check passed
default "Build #90 succeeded on Jenkins"
Details
@jamiecobbett jamiecobbett deleted the save-modal branch Feb 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.