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

Add gray box to My PL page with links #58182

Merged

Conversation

bethanyaconnor
Copy link
Contributor

@bethanyaconnor bethanyaconnor commented Apr 23, 2024

Finishes https://codedotorg.atlassian.net/browse/ACQ-1429 and moves links from the teacher homepage to the Professional Landing page.

Admin View:
Screenshot 2024-04-22 at 11 13 15 AM

Workshop Admin view:
Screenshot 2024-04-22 at 11 14 44 AM

@bethanyaconnor bethanyaconnor marked this pull request as ready for review April 24, 2024 14:22
@bethanyaconnor bethanyaconnor requested a review from a team April 24, 2024 14:22
@@ -23,22 +23,10 @@
#homepage-container

#workshop-links.container.main
-if can? :read, Pd::Application::ApplicationBase
Copy link
Contributor

Choose a reason for hiding this comment

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

These links should to more than Workshop Admins and until the work to add the Links for RPs, Facilitators and Workshop organizers is done I don't think we can remove

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see what you did. You are allows the same people to see the gray box. I don't think we want RPs, Facilitators and Workshop Organizers to see that gray box. Just internal Workshop Admins

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh woops -- I misunderstood! I'll move this PR back to draft and update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dmcavoy circling back to this. In the spec, these are the links listed to move to the gray box. I see that the application dashboard, workshop dashboard, etc are moving to other places as well. Should those links not be in the gray box at all?

Copy link
Contributor

Choose a reason for hiding this comment

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

No issues with the links. I think you have the right list of links. What I was commenting on was the permissions. As a whole this gray box should only show for Workshop Admins (internal Code.org staff who manage workshop and application stuff). Workshop Organizers, Regional Partners, Facilitators, etc will all have access to these links right on the page. Does that help?

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 think so!

@bethanyaconnor bethanyaconnor marked this pull request as draft April 24, 2024 16:22
@@ -0,0 +1,17 @@
- extra_links = []


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 decided to keep the conditions for which links are shown the same as they are on the homepage, but limit this box to admins and workshop admins. Likely some redundancy here, so I'm open to push back

@bethanyaconnor bethanyaconnor marked this pull request as ready for review May 3, 2024 13:49
@bethanyaconnor bethanyaconnor requested review from dmcavoy and a team May 3, 2024 13:49
Copy link
Contributor

@TurnerRiley TurnerRiley left a comment

Choose a reason for hiding this comment

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

Looks great! I think the redundancy seems fine especially since it's covered by the great tests you added!

@bethanyaconnor bethanyaconnor merged commit 4fb64db into staging May 6, 2024
2 checks passed
@bethanyaconnor bethanyaconnor deleted the bethany/pl-discoverability/move-links-to-gray-box branch May 6, 2024 15:37
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