Context Navigation

← Previous Ticket
Next Ticket →

#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:

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

Attachment trac_7310-modals.patch added

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

Attachment trac_7310-modals.2.patch added

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

Changed 11 years ago by timdumol

Attachment trac_7310-modals.3.patch added

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

Attachment trac_7310-modals.4.patch added

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: ↓ 5 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

Attachment trac_7310-modals.5.patch added

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

Attachment trac_7310-modals.6.patch added

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

Changed 11 years ago by mpatel

Attachment trac_7310-modals.7.patch added

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.