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

Improve UX when user's try and edit non-editable resources #3429

Closed
pq opened this issue Jun 7, 2012 · 8 comments
Closed

Improve UX when user's try and edit non-editable resources #3429

pq opened this issue Jun 7, 2012 · 8 comments
Assignees
Labels
type-enhancement A request for a change that isn't a bug
Milestone

Comments

@pq
Copy link
Member

pq commented Jun 7, 2012

What steps will reproduce the problem?

  1. Open samples/swarm
  2. Open an editor on SliderMenu.dart
  3. Edit and attempt to save

What is the expected output? What do you see instead?

The file is damaged but not saveable. You'd expect it either to be truly readonly (and so not damageable) or actually writable.

NOTE: SliderMenu.dart is in the ui_lib which is in a sibling folder to swarm and so is not in this case residing in the workspace.

Besides the obvious that locked files just aren't, a few quick thoughts:

* the lock icon is not dramatic enough; it's easily overlooked

  • the lock icon does not communicate enough --- e.g., it would be nice to know that this is an "external" file or an SDK lib file and so for that reason readonly
@pq
Copy link
Member Author

pq commented Jun 7, 2012

OK, with a little poking around I figured out why the files are quasi-editable. The issue is that ExternalCompilationUnitEditorInput has been wired to always be modifiable:

com.google.dart.tools.ui.internal.text.editor.ExternalCompilationUnitEditorInput {

  public boolean isModifiable() {
    // TODO (danrubel) Investigate implementing or removing this method in future CL
    return true;
  }
}

The good news is Dan was nice enough to leave some fingerprints so I know who to blame... :)

The CL that introduced this change is

http://codereview.chromium.org//8631010

More from Dan soon (I hope ;))

  


Set owner to @pq.
Added Accepted label.

@keertip
Copy link
Contributor

keertip commented Jun 7, 2012

Just a note on read only/editable issue. We do want the bundled libraries to be read only, but what about imported user libraries? My guess is that they should be editable. Both bundled and user defined libraries are opened as ExternalCompilationUnit.

@pq
Copy link
Member Author

pq commented Jun 7, 2012

I'm not actually sure... Do we want files outside the workspace to be editable? I haven't thought it through but my gut was we did not since I was assuming this would break assumptions made by analysis. But maybe I'm wrong!

@bwilkerson
Copy link
Member

It would break at least one assumption. Because they are not represented by an IResource there would not be any resource change notification to cause them to be re-analyzed. (Of course we already have that problem if the user edits them outside of Editor, but that's a separate issue.)

@keertip
Copy link
Contributor

keertip commented Jun 7, 2012

I would want the ability to tweak the libraries as I am working on the application to suit my needs - maybe the solution is that they should be opened in the editor to do so.

And for files not in the workspace we should make it truly readonly so that users do not get into a quasi state.

@pq
Copy link
Member Author

pq commented Jun 8, 2012

The burning issue (external files eronneously appearing as quasi writable) is addressed by: http://codereview.chromium.org/10536079/.

Remaining is the larger question of UX.

Reassigning to Luke for input.


Set owner to @lukechurch.
Removed Type-Defect label.
Added Type-Enhancement, Usability, Triaged labels.
Changed the title to: "Improve UX when user's try and edit non-editable resources".

@danrubel
Copy link

CL looks good as it solves the underlying problem that read-only files should not be editable. Unfortunately, it disables code completion in read-only files, but perhaps we can find another way to get code completion working there.

@danrubel
Copy link

danrubel commented Aug 3, 2012

Added Fixed label.

@pq pq added this to the M1 milestone Aug 3, 2012
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed type-enhancement labels Mar 1, 2016
copybara-service bot pushed a commit that referenced this issue Jun 7, 2023
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/0c8feac..449478c):
  449478c7  Tue Jun 6 20:04:19 2023 -0700  Sam Rawlins  Search: specify package rank in generation; distinguish non-core Dart libs (#3427)
  81af1bf3  Tue Jun 6 17:31:02 2023 -0700  Janice Collins  Prepare for dartdoc 6.3.0. (#3430)
  2b7340d3  Tue Jun 6 17:30:40 2023 -0700  Sam Rawlins  Explicitly export Kind; for internal customer (#3431)
  896540c0  Mon Jun 5 19:39:10 2023 +0000  dependabot[bot]  Bump github/codeql-action from 2.3.5 to 2.3.6 (#3429)

lints (https://github.com/dart-lang/lints/compare/fc74ce0..4b79906):
  4b79906  Tue Jun 6 15:15:22 2023 -0500  Parker Lougheed  Link to 'dart fix' docs in table header (#132)
  b7766d6  Tue Jun 6 09:05:20 2023 -0700  Devon Carew  add 'has quick-fix' checkboxes to the package's readme (#131)

native (https://github.com/dart-lang/native/compare/c851e69..973f3ed):
  973f3ed  Tue Jun 6 10:26:58 2023 +0000  Daco Harkes  [native_assets_cli] Add `dry_run` option (#59)

webdev (https://github.com/dart-lang/webdev/compare/b10d62b..3d7f546):
  3d7f546f  Mon Jun 5 16:24:16 2023 -0700  Elliott Brooks  Add Webdev installation test (#1935)
  9297c663  Mon Jun 5 14:31:43 2023 -0700  Elliott Brooks  Remove unnecessary `async` keywords(#2130)
  6b112b06  Mon Jun 5 12:45:34 2023 -0700  Elliott Brooks  Run DCM workflow against PR branch (#2131)

Change-Id: I6728516c13c1d45f8bbd65c266076c6028ed45ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/307962
Reviewed-by: Devon Carew <[email protected]>
Auto-Submit: Janice Collins <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
Commit-Queue: Janice Collins <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants