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

Refactor ui naming #134

Merged

Conversation

seanjyjy
Copy link

@seanjyjy seanjyjy commented Oct 6, 2020

Refactor the Ui naming to suit our new names -> Application, Company, Profile.

@seanjyjy seanjyjy added this to the v1.2 milestone Oct 6, 2020
@seanjyjy seanjyjy self-assigned this Oct 6, 2020
@seanjyjy seanjyjy linked an issue Oct 6, 2020 that may be closed by this pull request
Copy link

@shawn-nyk shawn-nyk left a comment

Choose a reason for hiding this comment

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

Fantastic work! Just one small question and we're good to merge (after the merge conflict is resolved)!

private static final String USER_INFORMATION_WIDTH = "-fx-min-width: 95";
private static final String COMPANY_INTERNSHIP_INFORMATION_WIDTH = "-fx-min-width: 75";
private static final String PROFILE_INFORMATION_WIDTH = "-fx-min-width: 95";
private static final String COMPANY_APPLICATION_INFORMATION_WIDTH = "-fx-min-width: 75";

Choose a reason for hiding this comment

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

Just to confirm, does company application refer to internship application i.e. our application class in this case?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the clarification Shawn, what I meant as this naming is, Company & Application to have the same Information Width. I will do the relevant changes to make it clearer.

Choose a reason for hiding this comment

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

Ah I see! Thanks for the clarification Sean!

…into refactor-ui-naming

� Conflicts:
�	src/main/java/seedu/address/ui/tabs/TabName.java
@codecov-io
Copy link

Codecov Report

Merging #134 into master will decrease coverage by 0.04%.
The diff coverage is 12.32%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #134      +/-   ##
============================================
- Coverage     44.65%   44.61%   -0.05%     
  Complexity      418      418              
============================================
  Files           125      126       +1     
  Lines          2076     2078       +2     
  Branches        229      228       -1     
============================================
  Hits            927      927              
- Misses         1102     1104       +2     
  Partials         47       47              
Impacted Files Coverage Δ Complexity Δ
.../main/java/seedu/address/commons/util/TabUtil.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...java/seedu/address/logic/commands/ExitCommand.java 100.00% <ø> (ø) 2.00 <0.00> (ø)
...va/seedu/address/logic/commands/SwitchCommand.java 87.50% <ø> (ø) 4.00 <0.00> (ø)
...in/java/seedu/address/logic/parser/ParserUtil.java 85.36% <0.00%> (+15.36%) 14.00 <0.00> (ø)
...eedu/address/logic/parser/SwitchCommandParser.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...rc/main/java/seedu/address/model/ModelManager.java 91.11% <ø> (ø) 21.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/ResultDisplay.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...n/java/seedu/address/ui/cards/ApplicationCard.java 0.00% <ø> (ø) 0.00 <0.00> (?)
src/main/java/seedu/address/ui/cards/Card.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
... and 13 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 6dd7cb4...2b8377b. Read the comment docs.

Copy link

@shawn-nyk shawn-nyk left a comment

Choose a reason for hiding this comment

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

LGTM!

@shawn-nyk shawn-nyk merged commit deafa7d into AY2021S1-CS2103T-T15-4:master Oct 7, 2020
@seanjyjy seanjyjy added the Code label Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor Ui naming
3 participants