Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#7310 closed defect (fixed)

Use modal dialogs instead of javascript prompts for the rename prompt.

Reported by: timdumol Owned by: boothby
Priority: major Milestone: sage-4.2.1
Component: notebook Keywords:
Cc: was, mpatel Merged in:
Authors: Tim Dumol Reviewers: Mitesh Patel
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Javascript prompts are jarring, and more importantly, are not properly handled by Selenium.

Attachments (9)

trac_7310-modals.patch (35.2 KB) - added by timdumol 11 years ago.
Replaces the javascript prompt with a modal dialog. Light dependency on #7309
trac_7310-modals.2.patch (35.4 KB) - added by timdumol 11 years ago.
Adds a 100ms fade in effect and prevents a warning under strict warnings mode (trailing comma).
trac_7310-modals.3.patch (36.4 KB) - added by timdumol 11 years ago.
Makes the rename dialog appear only when the page is fully loaded.
trac_7310-modals.4.patch (34.3 KB) - added by timdumol 11 years ago.
Adds a new function modal_prompt(...) to replace prompt() dialogs based on jQuery UI Dialog.
trac_7310-modals.5.patch (35.2 KB) - added by timdumol 11 years ago.
Adds the requisite imports to worksheet_listing.html
trac_7310-modals.6.patch (35.8 KB) - added by timdumol 11 years ago.
Removes the dialog from the DOM if destroy option is given -- this is done for performance and efficiency.
trac_7310-modals.7.patch (52.3 KB) - added by mpatel 11 years ago.
Minified three JS files. Various small changes. Apply only this patch.
trac_7310-modals.8.patch (52.3 KB) - added by mpatel 11 years ago.
Pre-select input text and use width: auto. Apply only this patch.
trac_7310-modals.8.2.patch (52.4 KB) - added by mpatel 11 years ago.

Download all attachments as: .zip

Change History (20)

Changed 11 years ago by timdumol

Replaces the javascript prompt with a modal dialog. Light dependency on #7309

comment:1 Changed 11 years ago by timdumol

  • Authors set to Tim Dumol
  • Status changed from new to needs_review

This patch relies on an additional body id introduced in #7309, but can easily be rebased if #7309 is not accepted.

Changed 11 years ago by timdumol

Adds a 100ms fade in effect and prevents a warning under strict warnings mode (trailing comma).

Changed 11 years ago by timdumol

Makes the rename dialog appear only when the page is fully loaded.

comment:2 Changed 11 years ago by mpatel

I tried v3, but the rename dialog was improperly rendered, possibly because an icon is missing? But why not use jQuery UI dialog, since we're already loading the library and it has built-in theme support?

I don't mean to pile on --- I'm happy to go through the Sage templates and code, converting alerts and prompts to dialogs.

Changed 11 years ago by timdumol

Adds a new function modal_prompt(...) to replace prompt() dialogs based on jQuery UI Dialog.

comment:3 Changed 11 years ago by timdumol

This should do the job. acking spotted only those two prompts. Alerts should probably be dealt in another ticket, if at all. This may be useful, if you're interested: http://boedesign.com/blog/2009/07/11/growl-for-jquery-gritter/.

comment:4 follow-up: Changed 11 years ago by mpatel

I'll try to take a closer look tomorrow. A quick note: The bgiframe plugin is already in sagenb/data/jqueryui/development-bundle/external.

Changed 11 years ago by timdumol

Adds the requisite imports to worksheet_listing.html

comment:5 in reply to: ↑ 4 Changed 11 years ago by timdumol

Replying to mpatel:

I'll try to take a closer look tomorrow. A quick note: The bgiframe plugin is already in sagenb/data/jqueryui/development-bundle/external.

Depending on the presence of the bgiframe plugin in the development kit (or the presence of the development kit itself) of an external package does not seem like a good idea to me.

Changed 11 years ago by timdumol

Removes the dialog from the DOM if destroy option is given -- this is done for performance and efficiency.

Changed 11 years ago by mpatel

Minified three JS files. Various small changes. Apply only this patch.

comment:6 Changed 11 years ago by mpatel

  • Reviewers set to Mitesh Patel

Version 7:

  • Updates a few JS functions per JSLint in "The Good Parts" mode.
  • Includes farbtastic.min.js, jquery.event.extendedclick.min.js, and jquery.form.min.js, all made with the YUI Compressor 2.4.2:
    java -jar yuicompressor-2.4.2.jar foo.js > foo.min.js
    
  • Uses src="/javascript/jquery/plugins/jquery.bgiframe.min.js" consistently.

To the extent it counts, my review is positive.

Changed 11 years ago by mpatel

Pre-select input text and use width: auto. Apply only this patch.

comment:7 Changed 11 years ago by mpatel

Version 8:

  • Uses width: auto for the input field.
  • Pre-selects the input text.

Changed 11 years ago by mpatel

comment:8 Changed 11 years ago by mpatel

Please use v8.2 instead of v8.

comment:9 Changed 11 years ago by was

  • Status changed from needs_review to positive_review

Extra positive review from me.

comment:10 Changed 11 years ago by was

  • Resolution set to fixed
  • Status changed from positive_review to closed

merged into sagenb-0.4.2 (sage-4.2.1)

comment:11 Changed 11 years ago by was

  • Milestone changed from sage-4.3 to sage-4.2.1
Note: See TracTickets for help on using tickets.