Opened 10 years ago
Closed 4 years ago
#9582 closed defect (fixed)
Invalid iconClassPrefix in TablePlugins.js and UploadImage.js
Reported by: | oliver_ | Owned by: | dylan |
---|---|---|---|
Priority: | high | Milestone: | 1.4 |
Component: | Dojox | Version: | 1.3.0 |
Keywords: | dijit editor buttons styles classes missing table image upload plugin | Cc: | |
Blocked By: | Blocking: |
Description
The Dojox Plugins TablePlugins? and UploadImage? have a wrong iconClassPrefix set.
Instead of iconClassPrefix:"editorIcon" it should be iconClassPrefix:"dijitEditorIcon" in line 214 of dojox/editor/plugins/TablePlugins.js and line 14 of dojox/editor/plugins/UploadImage.js, respectively.
This bug causes the icons for the buttons of these plugins (in the Dijit Editor) to be absent in Dojo Version 1.3.0 due to the lack of a editorIcon CSS class.
Change History (9)
comment:1 Changed 10 years ago by
Milestone: | tbd → 1.4 |
---|---|
Owner: | changed from Adam Peller to dante |
Status: | new → assigned |
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
yep, in [19067] -- postCommit hooks must be b0rked. sorry about that.
comment:4 Changed 10 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Actually I'm not sure whether this change is a good idea. Having added more buttons from the table plugin, I noticed they were absent too, which prompted me to dig deeper into the code which made me realize that there is only a class for dijitEditorIconInsertTable, but not for dijitEditorIconModifyTable and so on. I found that for the plugins, there is a separate CSS file existing in dojox/editor/plugins/resources/editorPlugins.css defining the needed classes editorIconInsertTable, editorIconModifyTable, ...
Sorry about that, I'm quite new to using Dojo and am not quite aware of the underlying structures yet. Since the editor is skinnable, I would have supposed it uses the skin CSS at dijit/themes instead of a separate CSS, where the icons stand out quite a bit from the rest of the design. So I suppose that's no problem at all, and reverting changes and loading the separate CSS should do. I'm sorry for the trouble!
comment:5 Changed 10 years ago by
Owner: | changed from dante to Mike Wilcox |
---|---|
Status: | reopened → new |
no trouble, this is a documentation fail. The css is linked in the two tests, but they don't follow ANY of our conventions (mixed tab/whitespace, tests are prefixed with test_, inline comments in the test, etc).
@mike, can you please clean this up. svn blame says you own him :)
comment:6 Changed 10 years ago by
Milestone: | 1.4 → future |
---|
comment:7 Changed 4 years ago by
Owner: | changed from Mike Wilcox to dylan |
---|---|
Status: | new → assigned |
comment:8 Changed 4 years ago by
Milestone: | future → 1.11 |
---|
comment:9 Changed 4 years ago by
Milestone: | 1.11 → 1.4 |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
Bug was fixed in 1.4. Ticket was left open to clean-up test code, which hasn't happened. If someone wants to clean that up, please open another ticket.
fixed?