Bugzilla – Full Text Bug Listing
| Summary: | Public API is needed to allow toggling of readOnly mode | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Mihai Sucan <mihai.sucan> |
| Component: | Editor | Assignee: | Felipe Heidrich <eclipse.felipe> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | eclipse.felipe, mihai.sucan, Silenio_Quarti |
| Version: | unspecified | ||
| Target Milestone: | 0.4 M1 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
Mihai Sucan
The readonly property became private because we add the getOptions() API. We plan to add setOptions() API which should solve this. Note that the iframe.body already have a class "viewContainer". (In reply to comment #0) > Recent changes to Orion have made the TextView.readonly property a private one, > _readonly. We have added TextView#getOptions() API to replace that.. > - please add a class name to the iframe body (or somewhere) to indicate the > document is readonly mode. can this be used instead ? var readonly = view.getOptions("readonly"); > > - please add a public API to allow us to change the read-only mode of Orion. > I personally would like to add a view.setOptions() that would allow the user to change readonly/fullselection/tabSize/expandTab/css etc With that you would be able to change the readonly mode and visual style (by passing in a new css set). Does that make sense to you ? (In reply to comment #2) > (In reply to comment #0) > > Recent changes to Orion have made the TextView.readonly property a private one, > > _readonly. > > > We have added TextView#getOptions() API to replace that.. Ah, I didn't notice. Thanks! > > - please add a class name to the iframe body (or somewhere) to indicate the > > document is readonly mode. > > can this be used instead ? > var readonly = view.getOptions("readonly"); This can be used, yes. But we do need setOptions(). > > - please add a public API to allow us to change the read-only mode of Orion. > > > > I personally would like to add a view.setOptions() that would allow the user to > change readonly/fullselection/tabSize/expandTab/css etc Sounds good to me! > With that you would be able to change the readonly mode and visual style (by > passing in a new css set). > > Does that make sense to you ? Changing the CSS like this is more cumbersome I believe. I think it would make sense to have .viewContainer.readOnly or something in our mozilla.css, so when the readonly state changes, the CSS automatically applies the visual change. It would be more complex to track form the integration script when readonly is active, add some CSS, then remove when readonly is false. This is really a matter of preference. Why I had this approach in mind: one can use in CSS input::disabled to change the visual style. It would be similarly nice/elegant and simple to implement .viewContainer.disabled. Thanks guys! (In reply to comment #3) > Changing the CSS like this is more cumbersome I believe. I think it would make > sense to have .viewContainer.readOnly or something in our mozilla.css, so when > the readonly state changes, the CSS automatically applies the visual change. That is a cool feature. But the main problem I would like to address at some point is the capability of changing the theme/skin for the entire editor, including styling for view, rulers, annotations, keywords, tooltips, etc See Bug 334212, Bug 347493 Do yo have any suggestions for this problem ? (In reply to comment #4) > (In reply to comment #3) > > Changing the CSS like this is more cumbersome I believe. I think it would make > > sense to have .viewContainer.readOnly or something in our mozilla.css, so when > > the readonly state changes, the CSS automatically applies the visual change. > > That is a cool feature. > But the main problem I would like to address at some point is the capability of > changing the theme/skin for the entire editor, including styling for > view, rulers, annotations, keywords, tooltips, etc > > See Bug 334212, Bug 347493 > > Do yo have any suggestions for this problem ? The larger problem is more complex than what I suggest. If you want I can submit a patch that gives me a public API for set/getReadOnly and does viewContainer.className += ' readOnly' ( and s/ readOnly// ) when needed. That would get us what we need, here and now. It's not a solution for the broad case, but it would work. This is what I am asking in this bug, if it's allowed until the better solution is implemented. General info: When we initialize Orion we give an array to the 'stylesheet' option: two files, orion.css and mozilla.css. The first file is a concatenation of all the small CSS files of Orion (we call this "upstream styling"). The second stylesheet is ours and we make small changes. In the update I am preparing we will continue to build orion.css but we will no longer use it, because some -webkit- selector has been added, which shows errors in our Error console. So, mozilla.css is going to always be a slightly-changed clone of orion.css. In the future we might theme the tokens more. To make theming easier I would suggest: - provide an orion.css style that we can always load, browser-neutral, nothing that causes errors in any modern browser (no vendor specific props/selectors). - keep only structural props and selectors, together with decent defaults for fonts/sizes/colors. - provide a orion-hacks.css file that would include all the vendor specific props/selectors from all browsers. This would be like now, but at least we could avoid integrating this stylesheet and we would put our own stuff in mozilla.css. - provide orion themes that give different fonts, sizes and colors to tokens and so on. - themes always override anything defined in orion.css. In such cases, we would integrate orion.css and probably a theme we'd like (or we'd make our own). We have added setOptions() to TextView but we are keeping this problem open till we find a solution for the css dance. (In reply to comment #6) > We have added setOptions() to TextView but we are keeping this problem open > till we find a solution for the css dance. Thank you very much! Proposed solution for the css dance: 1. add an event for when setOptions() is called, like OptionChanged, for every changed option. Such that consumers of the TextView can track option changes. 2. add a default event listener in textview for OptionChanged, for the "readonly" option. In the event handler change clientDiv.className to add/remove the readonly class. Should be fairly simple. (In reply to comment #8) > (In reply to comment #6) > > We have added setOptions() to TextView but we are keeping this problem open > > till we find a solution for the css dance. > > Thank you very much! > > Proposed solution for the css dance: > > 1. add an event for when setOptions() is called, like OptionChanged, for every > changed option. Such that consumers of the TextView can track option changes. > > 2. add a default event listener in textview for OptionChanged, for the > "readonly" option. In the event handler change clientDiv.className to > add/remove the readonly class. > The event is a good idea, but I think you might not need it after all. You are adding more options to setOptions that will allow you to change readOnly, className, and css in one call. For example view.setOptions({ readOnly: true, themeClass: 'readonly', stylesheet: css.concat(["path/readonly.css"]) } This will add readonly.css to the iframe, change the className of the body to "viewContainer readonly", and make the view readonly. In readonly.css you can redefine any style you want, for example, to change the line caret to yellow you could use .readonly .line_caret { background-color: yellow; } Does that make API make sense to you ? (In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #6) > > > We have added setOptions() to TextView but we are keeping this problem open > > > till we find a solution for the css dance. > > > > Thank you very much! > > > > Proposed solution for the css dance: > > > > 1. add an event for when setOptions() is called, like OptionChanged, for every > > changed option. Such that consumers of the TextView can track option changes. > > > > 2. add a default event listener in textview for OptionChanged, for the > > "readonly" option. In the event handler change clientDiv.className to > > add/remove the readonly class. > > > > The event is a good idea, but I think you might not need it after all. > You are adding more options to setOptions that will allow you to change > readOnly, className, and css in one call. For example > > > view.setOptions({ > readOnly: true, > themeClass: 'readonly', > stylesheet: css.concat(["path/readonly.css"]) > } > > This will add readonly.css to the iframe, change the className of the body to > "viewContainer readonly", and make the view readonly. > In readonly.css you can redefine any style you want, for example, to change the > line caret to yellow you could use > .readonly .line_caret { > background-color: yellow; > } > > > Does that make API make sense to you ? Yes it does! The themeClass option name seems a bit confusing. Maybe bodyClass? Or viewClassName? The option to update the stylesheet sounds great as well, but for a simple readonly change I will probably not use it now. I'll just include in my main stylesheet .readonly { whatever } and call view.setOptions({readOnly:true, viewClassName: 'readonly' }). Thank you! Hi Mihai, we just pushed the code http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=49d44455e92dd267d307e13b5cd1aa38ba0624b4 Let me know if it is working for you. Thank you. We can improve the name if you want. (In reply to comment #11) > Hi Mihai, we just pushed the code > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=49d44455e92dd267d307e13b5cd1aa38ba0624b4 > > Let me know if it is working for you. Thank you. > We can improve the name if you want. Tested. All works fine for me. Thank you very much! |