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

feat: Add internal portal wrapper to search cards #3666

Merged
merged 4 commits into from
Sep 13, 2024

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Sep 12, 2024

What does this PR do?

image

@DafyddLlyr DafyddLlyr force-pushed the dp/interal-portal-ui-wrapper branch from 520a5c9 to f3cae5b Compare September 12, 2024 18:58
Copy link

github-actions bot commented Sep 12, 2024

Removed vultr server and associated DNS entries

DafyddLlyr added a commit to theopensystemslab/planx-core that referenced this pull request Sep 13, 2024
Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

Functionally works exactly as expected !

A few small styling nits, please take them or leave them!

@@ -6,6 +6,7 @@ import CloudUpload from "@mui/icons-material/CloudUpload";
import ContactPage from "@mui/icons-material/ContactPage";
import CopyAll from "@mui/icons-material/CopyAll";
import Create from "@mui/icons-material/Create";
import DoorFrontOutlined from "@mui/icons-material/DoorFrontOutlined";
Copy link
Member

Choose a reason for hiding this comment

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

nit: could a simple folder icon be a more straightforward option here to communicate "this is nested inside something else"? Took me a while to make out exactly what the door was as the size it's used.

Maybe also more in line with future "what do we call things" conversations where internal portals become "folders" and external portals become "subscribed flows" or similar!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep this is a fair question!

I'm just re-using and formalising the existing icon here, as seen in the flow graph.

image

I'll keep as-is for now, but let's re-evaluate this in line with our "naming things" chat 👍

@DafyddLlyr DafyddLlyr merged commit ffcd3d6 into main Sep 13, 2024
12 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/interal-portal-ui-wrapper branch September 13, 2024 08:09
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.

2 participants