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

Change CSS to match theme in README #164

Merged
merged 31 commits into from
Oct 21, 2020

Conversation

LeeEnHao
Copy link
Member

@LeeEnHao LeeEnHao commented Oct 15, 2020

Description

Change CSS: slight modifications to existing CSS classes.
Result display reverted back to rectangle. Reason unable to change scroll pane background color, which will be exposed if content has a round radius.

Fixes #124

Testing

Manual Testing

  • Change result display message.
  • Result display unable to implement border radius property without switching away from TextArea to TextField.
    • Using TextField causes text to be un-scrollable if result message is too long.
  • Test suites for all ui components in separate pr

Remarks

Add any additional remarks for others to take note of.

@codecov-io
Copy link

codecov-io commented Oct 15, 2020

Codecov Report

Merging #164 into master will decrease coverage by 1.04%.
The diff coverage is 24.70%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #164      +/-   ##
============================================
- Coverage     74.22%   73.18%   -1.05%     
- Complexity      616      625       +9     
============================================
  Files            96       97       +1     
  Lines          1905     1939      +34     
  Branches        207      208       +1     
============================================
+ Hits           1414     1419       +5     
- Misses          425      446      +21     
- Partials         66       74       +8     
Impacted Files Coverage Δ Complexity Δ
src/main/java/seedu/address/MainApp.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...in/java/seedu/address/commons/core/LogsCenter.java 78.37% <ø> (ø) 10.00 <0.00> (ø)
src/main/java/seedu/address/ui/MainWindow.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
src/main/java/seedu/address/ui/TextClock.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
src/main/java/seedu/address/ui/WidgetViewBox.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../java/seedu/address/model/widget/WidgetObject.java 38.46% <10.00%> (+6.20%) 14.00 <9.00> (+8.00)
...seedu/address/model/widget/WidgetModelManager.java 52.17% <63.63%> (-0.46%) 6.00 <4.00> (+1.00) ⬇️
...va/seedu/address/logic/commands/CommandResult.java 84.21% <75.00%> (-4.03%) 10.00 <2.00> (ø)
...eedu/address/logic/commands/ClientViewCommand.java 100.00% <100.00%> (ø) 7.00 <0.00> (ø)
...java/seedu/address/logic/commands/ExitCommand.java 100.00% <100.00%> (ø) 2.00 <1.00> (ø)
... and 2 more

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 b421520...0c07d00. Read the comment docs.

@LeeEnHao LeeEnHao force-pushed the view-gui-feature branch 4 times, most recently from 4881205 to 2612e2a Compare October 18, 2020 18:09
@LeeEnHao LeeEnHao marked this pull request as ready for review October 18, 2020 18:13
@LeeEnHao
Copy link
Member Author

Please pull this down and test it locally. Especially on Ubuntu or macOs.

Copy link
Member

@qwoprocks qwoprocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI overall works as expected, just that the default message with div1 div2 can be seen at certain points of the program, such as when you run the app and type list.

Also, the view window doesn't update when you execute more commands, for example if I type client view 1 then client note add ..., or client edit ..., the view part doesn't reflect the changes.

src/main/java/seedu/address/MainApp.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/MainApp.java Outdated Show resolved Hide resolved
src/main/resources/view/DarkTheme.css Show resolved Hide resolved
src/main/resources/view/HelpWindow.fxml Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
List command re renders the view box.
Fix implemented: In line with the original implementation that
some command results which have a direct interaction with the view
have a boolean getter, view commands now has a boolean value view
to indicate to the view that the viewbox has to be updated/re-rendered
Add runtime dependency; junit jupiter engine.
Removed rendering of client notes from widget view box.
Reduced border radius of client list view and widget to reduce contrast
with the command result display box.
@LeeEnHao LeeEnHao self-assigned this Oct 20, 2020
@LeeEnHao
Copy link
Member Author

UI overall works as expected, just that the default message with div1 div2 can be seen at certain points of the program, such as when you run the app and type list.

Also, the view window doesn't update when you execute more commands, for example if I type client view 1 then client note add ..., or client edit ..., the view part doesn't reflect the changes.

I'll open a new issue for that as it could possibly be a non trivial implementation. Currently, the widget only updates on the being given a view command.

Copy link
Member

@qwoprocks qwoprocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small stuff, other than those, I think LGTM

src/main/resources/view/MainWindow.fxml Outdated Show resolved Hide resolved
src/main/resources/view/MainWindow.fxml Outdated Show resolved Hide resolved
src/main/resources/view/WidgetViewBox.fxml Outdated Show resolved Hide resolved
Copy link
Member

@rtshkmr rtshkmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for now because it's functional. Here are some point pointers to take note of for a follow up PR:

  • relevant people need to update the help window (command description). also help window should have some sort of scrollbar/diff dimensions cuz the command list gonna be long for the current dimensions

  • fix Locale issue for default display, maybe we create a bug report for that. But leave it as is for Friday's demo. I

image

/**
* Stops the clock.
*/
public void stop() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an unused method for now, can't see where else it's used..

textThree.setText("");
divThree.setText("");
textFour.setText("");
footer.setText("Made in NUS");
Copy link
Member

@rtshkmr rtshkmr Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Footer should probably be lowered to the bottom border right.

But fixing this isn't that impt for now since it's functional!

image

@LeeEnHao LeeEnHao merged commit ebe5f29 into AY2021S1-CS2103T-F11-4:master Oct 21, 2020
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.

Restructure GUI to include View Box widget
5 participants