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

Fixed: Selected entry count in journal could not be updated after erasing a selected entry from the palette. #743

Closed
wants to merge 2 commits into from

Conversation

zeeshank95
Copy link

This is the link to the bug, https://bugs.sugarlabs.org/ticket/4972#comment:1
When we select multiple files in the journal and if we try to remove a single file by double clicking on the file icon and selecting "erase" option from the palette then the file will be erased but the selected entry count could not get updated. Selecting multiple files and trying to remove a single file from the palette, it does not make any sense and also we have an "erase" option in the toolbar which can be used for erasing multiple files. So when multiple files are selected the palette should not display any "erase" option in the palette, when we double click on the file icon, but it will show the erase option in the palette when no file is selected so that we can remove a single file from the palette only.
There will be no issue of the selected entry count, the count will be updated correctly.

…asing a selected entry from palette.

This is the link to the bug, https://bugs.sugarlabs.org/ticket/4972#comment:1
When we select multiple files in the journal and if we try to remove a single file by double clicking on the file icon and selecting "erase" option from the palette then the file will be erased but the selected entry count could not get updated. Selecting multiple files and trying to remove a single file from the palette, it does not make any sense and also we have an "erase" option in the toolbar which can be used for erasing multiple files. So when multiple files are selected the palette should not display any "erase" option in the palette, when we double click on the file icon, but it will show the erase option in the palette when no file is selected so that we can remove a single file from the palette only.
There will be no issue of the selected entry count, the count will be updated correctly.
Fixed : Selected entry count in journal could not be updated after erasing a selected entry from the palette.
@zeeshank95 zeeshank95 changed the title Fixed :Selected entry count in journal could not be updated after erasing a selected entry from the palette. Fixed: Selected entry count in journal could not be updated after erasing a selected entry from the palette. Feb 22, 2017
@quozl
Copy link
Contributor

quozl commented Feb 22, 2017

While this will fix the problem, it does so in a way that hides a feature; journal entry erase. Yet apart from complexity of fix, there's no reason to hide that feature.

Could you instead deselect an entry before erase?

Also, you have a merge commit in the pull request. Please remove it.

@quozl
Copy link
Contributor

quozl commented Feb 22, 2017

Also, for your interest, your reference to self._journalactivity._editing_mode violates our coding style; as _editing_mode is a private variable; it should be get_editing_mode() instead. By mentioning this I'm not agreeing with the patch, I'm just trying to help you with coding style in future. Reference: PEP 8, Designing for inheritance.

@godiard
Copy link
Contributor

godiard commented Feb 22, 2017

If the journal is in the editing_mode, the individual palettes should not be opened at all.

@quozl
Copy link
Contributor

quozl commented Feb 23, 2017

@godiard wrote:

If the journal is in the editing_mode, the individual palettes should not be opened at all.

Yes, if it were not for our users who have discovered they can use the individual palettes. It is too late for them, and for us to seek the perfect UX now would break what they have discovered.

@godiard
Copy link
Contributor

godiard commented Feb 23, 2017

Using the individual palettes will break in other ways too, by example cloning a item....

@quozl
Copy link
Contributor

quozl commented Feb 23, 2017

Good point, that may have to be handled as well. I've tested, and Duplicate does not cause a problem. Message changed from "Selected 1 of 4" to "Selected 1 of 5". Duplicate entry did not join the selection. Apparently harmless.

A use case is gradual build-up of a selection for a mass Copy or Erase while also resuming, send to project, copying, or erasing individual entries.

@zeeshank95
Copy link
Author

@quozl
I can try to deselect an entry before erase. It will solve the problem for 'erase' at least, right?

@quozl
Copy link
Contributor

quozl commented Feb 23, 2017

@zeecoder606, it is easier for me to do it than to explain it. Please review and test #744.

@quozl
Copy link
Contributor

quozl commented Apr 21, 2017

Replaced by #744.

@quozl quozl closed this Apr 21, 2017
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

3 participants