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: modify permissions tab UI #790

Merged
merged 6 commits into from
Jan 12, 2021
Merged

Fix: modify permissions tab UI #790

merged 6 commits into from
Jan 12, 2021

Conversation

ElinorW
Copy link
Contributor

@ElinorW ElinorW commented Jan 11, 2021

Overview

  • Remove 'display string' column
  • Wrap information contained under 'Description' column
  • Change description message dependent on whether a user has signed in or not

Fixes #653

Demo

image

Notes

Optional. Ancillary topics, caveats, alternative strategies that didn't work out, anything else.

Testing Instructions

  • How to test this PR
  • Prefer bulleted description
  • Start after checking out this branch
  • Include any setup required, such as bundling scripts, restarting services, etc.
  • Include test case, and expected output

@ElinorW ElinorW requested a review from thewahome January 11, 2021 12:56
}

const TabList = ({ permissions, columns, classes, renderItemColumn, renderDetailsHeader }: ITabList) => {

const TabList = ({ permissions, columns, classes, renderItemColumn, renderDetailsHeader, permissionToken }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using the useSelector React Hook may help to reduce passing too many arguments into the TabList component since it has access to the redux state. This can be used to pick up whether the user is logged in or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay perfect! Thanks for this!

@@ -306,7 +306,7 @@
"Snippet not available": "Snippet not available",
"Select different permissions": "To try out different Microsoft Graph API endpoints, choose the permissions, and then click Consent.",
"Search permissions": "Search permissions",
"permissions not found": "We did not find permissions",
"permissions not found": "We did not find permissions. To view a list of all available permissions, click on the Settings gearbox and then click on Select Permissions.",
Copy link

Choose a reason for hiding this comment

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

is gearbox a thing people understand? just never seen it before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will rename that to settings icon

@jobala
Copy link
Contributor

jobala commented Jan 12, 2021

Admin consent column is off.

image

@ElinorW ElinorW merged commit dcefa4b into dev Jan 12, 2021
thewahome added a commit that referenced this pull request Jan 30, 2021
* Fix - replaces content-type header by accept to match HTTP and avoid CORS pre-flight (#783)

* Fix: modify permissions tab UI (#790)

* Track errors  (#777)

* Fix: sanitize url when fetching permissions (#794)

* Fix: remove wrongly placed working (#795)

* Fix: all permissions show as required (#797)

* Task: autocomplete hover styling (#801)

* Fix: Enable screen reader confirmation feedback (#802)

* Migrate to eslint (#627)

* Feature: resizable components (#766)

* Fix: add onItemInvoked action (#806)

* Fix: permissions consent (#807)

* Task: accessibility ci (#358)

* Fix: prevent resize when view expanded (#816)

* Feature: additional telemetry (#813)

* Fix: permissions tab UI (#815)

* Fix: shrink request section (#822)

Co-authored-by: jobala <japhethobalak@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: Elinor <ekaguongo@gmail.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>
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.

Improve user experience on the "Modify permissions" tab
4 participants