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

platform dashboard #202

Merged
merged 7 commits into from
Oct 24, 2024
Merged

Conversation

kialj876
Copy link
Collaborator

@kialj876 kialj876 commented Oct 21, 2024

Issue:

  • bcgov/entity/issues/

Description of changes:

  • Renaming of connect form components is an ask from the connect repo PR (let me know if this is in line with what you were thinking)
  • dashboard work is in progress (several things I still need to do), but I was hoping you'd have a look at the way I'm doing the 'DetailsHeader' in the layout and setting the values via a connect store. Was thinking we would update the button control / breadcrumbs to do it this way as well instead of adding custom page meta values, but have a look and see if you like it
  • new page path: .../platform/registration/:regId

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the namex license (Apache 2.0).

@kialj876 kialj876 marked this pull request as draft October 21, 2024 21:15
@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://strr-platform-dev--pr-202-kinxn4mf.web.app

@deetz99
Copy link
Collaborator

deetz99 commented Oct 22, 2024

  1. component renaming looks good to me

  2. This is something I'm still mulling over myself, what the best way to go about this is (including breadcrumbs/button control). One thing to keep in mind is if different people are using the layer we might want to add some flexibility. For example, do we add props to all these components, and set the props in the layout files using the route meta (if people want to make custom layouts, we aren't forcing them to use the route meta this way) or do we get rid of the page meta entirely and only rely on props/store values?. One thought I had was to add props to the components and keep the setBreadcrumbs/setButtonControl utils and then also have a store usePageMetaStore where we would handle all the state utilizing the page meta utils we have. This way we could allow only props or only page meta or only store values to manage the state. Downside is we would have to pass the values in whenever creating a new layout and maybe it's overkill and we should be forcing people to use it in a certain way?. Anyways, I don't see any issue with using the store for strr but we'll want to think about this if pulling it into the layer.

  3. 👍

@kialj876 kialj876 force-pushed the platform-web-form-updates-5 branch from 9273257 to 9602869 Compare October 23, 2024 21:07
@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://strr-platform-dev--pr-202-kinxn4mf.web.app

@kialj876 kialj876 force-pushed the platform-web-form-updates-5 branch from 9602869 to a326b5c Compare October 23, 2024 21:15
@kialj876 kialj876 marked this pull request as ready for review October 23, 2024 21:15
@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://strr-platform-dev--pr-202-kinxn4mf.web.app

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://strr-platform-dev--pr-202-kinxn4mf.web.app

Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
@kialj876 kialj876 force-pushed the platform-web-form-updates-5 branch from 9317bbf to 42ea9da Compare October 24, 2024 21:18
@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://strr-platform-dev--pr-202-kinxn4mf.web.app

@kialj876 kialj876 changed the title Interim dashboard draft commit platform dashboard Oct 24, 2024
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Copy link

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://strr-platform-dev--pr-202-kinxn4mf.web.app

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this correct? 'text-lg' is 18px

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think so? We've got H1's as 1.25rem (20px) and H2's as 1.125rem (18px) in many places. The explicit '-32px' / '-24px' were originally from the strr-web project

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could be wrong about this for sure though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay after looking at the other apps I think this is supposed to be 2rem for H1 and 1.5rem for H2, and then we can just override it for other cases where it varies

Copy link
Collaborator

@deetz99 deetz99 left a comment

Choose a reason for hiding this comment

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

Just a few minor comments but everything looks great!

@thorwolpert thorwolpert merged commit 095ae3f into bcgov:main Oct 24, 2024
11 of 12 checks passed
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.

4 participants