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

Display read-only state of open files using a lock icon #3924

Merged
merged 1 commit into from
May 24, 2024

Conversation

bjorn
Copy link
Member

@bjorn bjorn commented Apr 15, 2024

This should be mostly helpful for people using a VCS like Perforce or Subversion, which may require a file to be checked out or locked before it can be edited.

For now the read-only icon is a lock, which is common, but it may be confusing for Subversion users since they actually need to lock the file to make it writable.

@bjorn bjorn force-pushed the display-read-only-status branch from e05d7a3 to ccbaba6 Compare May 24, 2024 15:47
This should be mostly helpful for people using a VCS like Perforce or
Subversion, which may require a file to be checked out or locked before
it can be edited.

For now the read-only icon is a lock, which is common, but it may be
confusing for Subversion users since they actually need to lock the file
to make it writable.
@bjorn bjorn force-pushed the display-read-only-status branch from ccbaba6 to c82539e Compare May 24, 2024 16:14
@bjorn bjorn merged commit c82539e into mapeditor:master May 24, 2024
13 of 14 checks passed
@bjorn bjorn deleted the display-read-only-status branch May 24, 2024 16:30
@eishiya
Copy link
Contributor

eishiya commented May 31, 2024

The lock is displayed for maps that have not been saved to a file, which doesn't feel appropriate. It makes some sense, the maps do have to be Saved As to write the changes since they can't be Saved normally, but presenting the file as locked/read-only doesn't seem a good fit.
image

I think it's been discussed before that Tiled probably needs to present unsaved files in some different way, e.g. "Unsaved map" instead of "untitled.tmx", which implies a file that doesn't exist. Perhaps an alternative is to use a different icon for unsaved documents, such as the new document icon.

@bjorn
Copy link
Member Author

bjorn commented May 31, 2024

The lock is displayed for maps that have not been saved to a file

Thanks for catching that! It actually happens because internally read-only is set to !file.isWritable(), which obviously will be true for files that do not exist. I'll extend that with a file exists check.

I agree the unsaved map could be better identified, though the use of "untitled" as file name for unsaved files seems to still be quite ubiquitous. What about just "New Map"?

bjorn added a commit to bjorn/tiled-dev that referenced this pull request May 31, 2024
bjorn added a commit to bjorn/tiled-dev that referenced this pull request May 31, 2024
@eishiya
Copy link
Contributor

eishiya commented May 31, 2024

"Untitled" as a file name is ubiquitous, yes, but there's no file, and therefore no file name xP "New Map" would work. Even "Untitled" might be fine, the main thing is to get rid of the file extension that implies there's a file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants