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: expand maxWidth allow more humanName chars #764

Merged
merged 11 commits into from
Feb 12, 2021

Conversation

ezkemboi
Copy link
Contributor

@ezkemboi ezkemboi commented Nov 16, 2020

Overview

There is issue #694 that was raised and Assigned in September. Since it has been a while since it was assigned and not done, I felt that I could provide a solution.

Demo

Screenshot 2020-11-16 at 20 56 01

Notes

-> Simple way was to expand or allow stretching of maxWidth for humanName column. [Solution I went with], which seems to fix the horizontal scrollbar(I was unable to reproduce).
-> Another way was to make calculations based on the screen window.innerWidth and factor in the 25% that is max-width for the sidebar. Somehow complex.

Testing Instructions

  • Pull the latest changes in the branch
  • Run the app and check on sidebar sections [human Name message]
  • Expected: it should not limit to 24 chars

@ezkemboi
Copy link
Contributor Author

@ElinorW and @thewahome you can have a look at this PR.

Copy link
Contributor

@ElinorW ElinorW 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 !

@thewahome thewahome self-requested a review November 17, 2020 09:28
Copy link
Collaborator

@thewahome thewahome left a comment

Choose a reason for hiding this comment

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

On smaller screens, the width of the human name column pushes the documentation link button to the right and makes it not visible.

Possibly making the documentation occupy a fixed distance from the scrollbar would fix thix

@ezkemboi
Copy link
Contributor Author

ezkemboi commented Nov 17, 2020

On smaller screens, the width of the human name column pushes the documentation link button to the right and makes it not visible.

Possibly making the documentation occupy a fixed distance from the scrollbar would fix thix

It was not easy getting a fixed distance for IColumn since when I passed currentWidth with no minWidth, it returned an error.
Instead, I realized maxWidth of humanName being 180 worked well with most screen except for larger screens more than 1280 width.
And I introduced a change of maxWidth based on the window screen width but making 180 as default.
I hope this approach is good @thewahome

@thewahome
Copy link
Collaborator

It was not easy getting a fixed distance for IColumn since when I passed currentWidth with no minWidth, it returned an error.
Instead, I realized maxWidth of humanName being 180 worked well with most screen except for larger screens more than 1280 width.
And I introduced a change of maxWidth based on the window screen width but making 180 as default.
I hope this approach is good @thewahome

Right now this implementation currently works on the very large screen but as I minimise, I notice that the documentation link gets lost. Which truly is a tricky thing to fix.
I think placing a listener on the window size would help us know for sure the size of the window so that we can create the width as a percentage of what we have as the sidebar width. What do you think @ezkemboi ?

@ezkemboi
Copy link
Contributor Author

It was not easy getting a fixed distance for IColumn since when I passed currentWidth with no minWidth, it returned an error.
Instead, I realized maxWidth of humanName being 180 worked well with most screen except for larger screens more than 1280 width.
And I introduced a change of maxWidth based on the window screen width but making 180 as default.
I hope this approach is good @thewahome

Right now this implementation currently works on the very large screen but as I minimise, I notice that the documentation link gets lost. Which truly is a tricky thing to fix.
I think placing a listener on the window size would help us know for sure the size of the window so that we can create the width as a percentage of what we have as the sidebar width. What do you think @ezkemboi ?

I think that will be a good idea.
I will be adding the listener on the same.

@thewahome thewahome merged commit 74d0401 into microsoftgraph:dev Feb 12, 2021
ElinorW added a commit that referenced this pull request Feb 16, 2021
* Fix: support different content types (#814)

* Feature: Add Adaptive cards  JSON Schema code (#828)

* Feature: samples testing (#833)

* Feature: Adds 'Report an Issue' menu option (#834)

* Feature: clickable links on message bar (#835)

* fix: intermittent create page error (#837)

* Fix: Add 'Maximize sidebar' aria-label (#842)

* Task: upgrade technologies (#844)

* Fix: expand maxWidth allow more humanName chars (#764)

* Fix: autocomplete options not displaying (#847)

* fix: adaptive cards destroy (#848)

* Fix: Graph Explorer link colors (#846)

* Fix: Adaptive cards instrumentation (#849)

* Fix: adjust column width (#850)

* Task: prevent storing access token (#851)

Co-authored-by: jobala <japhethobalak@gmail.com>
Co-authored-by: Charles Wahome <thewahome.cw@gmail.com>
Co-authored-by: OfficeGlobal <47977325+OfficeGlobal@users.noreply.github.com>
Co-authored-by: OfficeGlobal <OfficeGlobal@microsoft.com>
Co-authored-by: Azure Static Web Apps <opensource@microsoft.com>
Co-authored-by: Millicent Achieng <achieng.milli@gmail.com>
Co-authored-by: Sébastien Levert <sebastienlevert@users.noreply.github.com>
Co-authored-by: Ezrqn Kemboi <ezrqnkemboi@gmail.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Joseph Ngugi <jngugi88@gmail.com>
Co-authored-by: Maggie Kimani <maggiekim42@gmail.com>
ElinorW added a commit that referenced this pull request Feb 22, 2021
* Fix: support different content types (#814)

* Feature: Add Adaptive cards  JSON Schema code (#828)

* Feature: samples testing (#833)

* Feature: Adds 'Report an Issue' menu option (#834)

* Feature: clickable links on message bar (#835)

* fix: intermittent create page error (#837)

* HB of localized GE.jsons (#830)

* Fix: Add 'Maximize sidebar' aria-label (#842)

* Task: upgrade technologies (#844)

* Fix: expand maxWidth allow more humanName chars (#764)

* Fix: autocomplete options not displaying (#847)

* fix: adaptive cards destroy (#848)

* Fix: Graph Explorer link colors (#846)

* Fix: Adaptive cards instrumentation (#849)

* Fix: adjust column width (#850)

* Task: prevent storing access token (#851)

* HB of localized GE.jsons (#854)

* chore(release): 4.3.0 (#857)

* Fix: rename component name (#859)

* chore(release): 4.4.0

Co-authored-by: jobala <japhethobalak@gmail.com>
Co-authored-by: Charles Wahome <thewahome.cw@gmail.com>
Co-authored-by: OfficeGlobal <47977325+OfficeGlobal@users.noreply.github.com>
Co-authored-by: OfficeGlobal <OfficeGlobal@microsoft.com>
Co-authored-by: Azure Static Web Apps <opensource@microsoft.com>
Co-authored-by: Millicent Achieng <achieng.milli@gmail.com>
Co-authored-by: Sébastien Levert <sebastienlevert@users.noreply.github.com>
Co-authored-by: Ezrqn Kemboi <ezrqnkemboi@gmail.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Joseph Ngugi <jngugi88@gmail.com>
Co-authored-by: Maggie Kimani <maggiekim42@gmail.com>
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.

samples text (humanName) gets truncated for no reason
3 participants