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 - #4685 - Project Details text fields not clobbered by label #5540

Merged
merged 16 commits into from Dec 10, 2018

Conversation

richmahn
Copy link
Member

@richmahn richmahn commented Dec 7, 2018

Describe what your pull request addresses (Please include relevant issue numbers):

Please include detailed Test instructions for your pull request:

  • Check out this branch, update your node-modules
  • Go to the Project Details page of a project. Now the page is more flexible in that if you have a narrow screen it will put all fields in one column, but max at two columns.
  • If the "Translation Identifier" or "Project Nickname" fields' text is too long (wraps to two line), the text will not flow into the text field (this only is true for text fields! Cannot control the select fields as they use absolute positioning to float their stuff. Needs Material-UI node module latest version to fix)

Standard Test Instructions for PR Review Process:

  • Double check unit tests that have been written
  • Check for documentation for code changes
  • Check that there are not inadvertent commits to tC Apps when reviewing a tC Core PR
  • Checkout the branch locally and ensure that app runs as expected
    • Ensure tests pass
    • Open and watch the console for errors
    • Make sure all actions perform as expected
    • Import and Load a new Project
    • Load a tool and perform basic actions
    • Switch tools and perform basic actions
    • Switch project to an existing project
    • Load a tool and perform basic actions
    • Switch tools and perform basic actions
    • Next time reverse the order of importing after loading an existing project
  • Reviewer should double check the DoD in the ISSUE, including the “spirit” of the story
  • Ask Ben or Koz if you have any concerns about implementation (especially UI related)

This change is Reviewable

PhotoNomad0
PhotoNomad0 previously approved these changes Dec 10, 2018
Copy link
Contributor

@PhotoNomad0 PhotoNomad0 left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 7 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@codecov
Copy link

codecov bot commented Dec 10, 2018

Codecov Report

Merging #5540 into develop will not change coverage.
The diff coverage is 12.5%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5540   +/-   ##
========================================
  Coverage    57.84%   57.84%           
========================================
  Files          265      265           
  Lines         8471     8471           
  Branches      1403     1403           
========================================
  Hits          4900     4900           
  Misses        3112     3112           
  Partials       459      459
Impacted Files Coverage Δ
...Validation/ProjectInformationCheck/CheckersArea.js 62.5% <ø> (ø) ⬆️
...ctValidation/ProjectInformationCheck/TextPrompt.js 87.5% <ø> (ø) ⬆️
...ion/ProjectInformationCheck/LanguageNameTextBox.js 100% <ø> (ø) ⬆️
...dation/ProjectInformationCheck/ContributorsArea.js 62.5% <ø> (ø) ⬆️
...ation/ProjectInformationCheck/LanguageIdTextBox.js 100% <ø> (ø) ⬆️
...projectValidation/ProjectInformationCheck/index.js 30.95% <12.5%> (ø) ⬆️

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 56c71c7...3f2a8d8. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 10, 2018

Codecov Report

Merging #5540 into develop will not change coverage.
The diff coverage is 12.5%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5540   +/-   ##
========================================
  Coverage    57.85%   57.85%           
========================================
  Files          265      265           
  Lines         8470     8470           
  Branches      1403     1403           
========================================
  Hits          4900     4900           
  Misses        3111     3111           
  Partials       459      459
Impacted Files Coverage Δ
...Validation/ProjectInformationCheck/CheckersArea.js 62.5% <ø> (ø) ⬆️
...ctValidation/ProjectInformationCheck/TextPrompt.js 87.5% <ø> (ø) ⬆️
...ion/ProjectInformationCheck/LanguageNameTextBox.js 100% <ø> (ø) ⬆️
...dation/ProjectInformationCheck/ContributorsArea.js 62.5% <ø> (ø) ⬆️
...ation/ProjectInformationCheck/LanguageIdTextBox.js 100% <ø> (ø) ⬆️
...projectValidation/ProjectInformationCheck/index.js 30.95% <12.5%> (ø) ⬆️

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 548eeb2...14bfd68. Read the comment docs.

Copy link
Contributor

@PhotoNomad0 PhotoNomad0 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@PhotoNomad0 PhotoNomad0 merged commit 4d06842 into develop Dec 10, 2018
@dsm-git
Copy link

dsm-git commented Dec 10, 2018

Linux build 1.1.0 (3f2a8d8) is ready for download.
macOS build 1.1.0 (3f2a8d8) is ready for download.
Windows x64 build 1.1.0 (3f2a8d8) is ready for download.
Windows x32 build 1.1.0 (3f2a8d8) is ready for download.

@richmahn richmahn deleted the fix-richmahn-4685 branch January 7, 2019 17:30
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