Skip to content
This repository has been archived by the owner on Feb 25, 2020. It is now read-only.

Fixes #147 - Resize line changes panel #316

Merged

Conversation

hhagenson28
Copy link
Collaborator

@hhagenson28 hhagenson28 commented Apr 11, 2019

Related Issue/Keyword:

Closes #147

Description:

After a lot of trial and error, I have been able to adjust the dimensions of the line changes panel so that:

  • all line changes are now visible and are not hidden behind the command console
  • the horizontal scroll bar is now visible for files with lengthy lines
  • the whole panel is wider for better readability of the line changes

In order to achieve this I had to restructure how the repository name and branch name are shown in the header. These are now limited to the top line of the header with ellipsis (...) for the overflow but you can hover over the text to see their full names. This was done to ensure that the sizing of the header, footer and body content remains consistent and nothing is being hidden behind anything else.

Making these changes will make issue #279 easier to achieve as well.

Testing:

Steps for manual testing:

  1. Open up a repo with modified files
  2. View the line changes of a modified file and check that you can see all of the changes for the following:
  • a one line change
  • multiple line changes (enough changes that the vertical scroll bar is active)
  • a change that occurs on a long line (in order to view the horizontal scroll bar)
  1. Ensure that you can see the full repo and branch names by hovering over the text for these

None of the lines in the panel should be hidden behind the command console.

Thank you to @SheepySean for your help with the CSS and providing very helpful solutions!

(Please ignore all of the commits to do with the package-lock file, I don't know what its problem is)

Checklist:

  • Latest master merged/rebased into your feature branch
  • Tests covering all changes
  • Meets the projects coding conventions
  • No out of scope changes
  • @Mentioned any relevant team members
  • No failure when running the linter (npm run lint) - errors but not related to code I changed for this PR
  • Included a relevant gif

…and added hover effect to see full names of both.
…and added hover effect to see full names of both.
@liamtbrand
Copy link
Collaborator

liamtbrand commented Apr 11, 2019

Note to reviewer(s): This is going to conflict with #315. Resolving these conflicts shouldn't be too bad, just have a look at the files changed and the reasons it should be pretty straight forward to resolve. Let me know if you need help.

Copy link
Collaborator

@SamuelZheng11 SamuelZheng11 left a comment

Choose a reason for hiding this comment

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

I have reviewed your file changes and they look clean :D, However graphical problem when I scroll across pass a certain point

explianiton

Is this expected?

Tested on Windows 10, Chrome

* Close diff panel on opening repository

- Refactors the diff panel component to contain the opening and closing methods.

* Fix linting issues
@hhagenson28
Copy link
Collaborator Author

@SamuelZheng11 Yes, this issue with the colours being cut off has been noted in issue #204 and therefore, is not related to my changes.

@SamuelZheng11
Copy link
Collaborator

@SamuelZheng11 Yes, this issue with the colours being cut off has been noted in issue #204 and therefore, is not related to my changes.

Ah Ok I approve then

@hhagenson28
Copy link
Collaborator Author

@SamuelZheng11 the conflicts have now been fixed so you can merge this in too!

Copy link
Collaborator

@SamuelZheng11 SamuelZheng11 left a comment

Choose a reason for hiding this comment

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

Looks good, Ive run it again my machine and still works.
Ive cross-referenced with PR #315 and almost all changes are accounted for
Ive left a comment regarding changes to styles.css

stylesheets/styles.css Outdated Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resize line changes panel
3 participants