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

Fix bug in details command #48

Merged

Conversation

yongxiangng
Copy link
Member

Code snippet causing the bug. command result is needed to know if the ui should show persons as details or not. however, when the result is received, the model is updated already (model updates during execution) and setting show details does not update the personlistpanel component, so we have to explicitly tell it to refresh

            CommandResult commandResult = logic.execute(commandText);
            logger.info("Result: " + commandResult.getFeedbackToUser());
            resultDisplay.setFeedbackToUser(commandResult.getFeedbackToUser());

            if (commandResult.isShowHelp()) {
                handleHelp();
            }

            if (commandResult.isExit()) {
                handleExit();
            }

            personListPanel.setShowDetails(commandResult.isShowDetails());

The cause of the bug was due to personListPanel not refreshing. Note that calling personListView.refresh(); is not sufficient to refresh the entire component. setting items and cell factory does a "refresh".

This closes #47 .

@yongxiangng yongxiangng added this to the v1.2 milestone Oct 11, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #48 (efff960) into master (4015615) will decrease coverage by 0.27%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #48      +/-   ##
============================================
- Coverage     70.65%   70.38%   -0.28%     
  Complexity      417      417              
============================================
  Files            75       75              
  Lines          1295     1300       +5     
  Branches        132      132              
============================================
  Hits            915      915              
- Misses          345      350       +5     
  Partials         35       35              
Impacted Files Coverage Δ
...rc/main/java/seedu/address/ui/PersonListPanel.java 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4015615...efff960. Read the comment docs.

@yongxiangng yongxiangng added severity.High A flaw that affects most users and causes major problems for users. i.e., makes the product almost u type.Bug A bug labels Oct 11, 2021
@nzixuan nzixuan merged commit 4cd5cd8 into AY2122S1-CS2103-T16-4:master Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity.High A flaw that affects most users and causes major problems for users. i.e., makes the product almost u type.Bug A bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in details command
3 participants