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 seedlot navigator in seedlot dashboard #1643

Merged
merged 4 commits into from
Oct 3, 2024

Conversation

xiaopeng0202
Copy link
Collaborator

@xiaopeng0202 xiaopeng0202 commented Sep 25, 2024

Description

Closes #1626

Changelog

New

  • added go to seedlot button in seedlot dashboard and my seedlot page

Changed

  • refactored and created seedlot navigator component

How was this tested?

  • 🧠 Not needed
  • 👀 Eyeball
  • 🤖 Added tests


Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are deployed in:


Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are deployed in:

@xiaopeng0202 xiaopeng0202 changed the title feat: add go to seedlot button in seedlot dashboard feat: add seedlot navigator in seedlot dashboard Sep 25, 2024
Copy link
Collaborator

@mgaseta mgaseta left a comment

Choose a reason for hiding this comment

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

I think this task was to add the seedlot navigator to "My Seedlots" page, not to the "Seedlot Dashboard", can you confirm @SLDonnelly and @kevinginley ?

@xiaopeng0202
Copy link
Collaborator Author

I think this task was to add the seedlot navigator to "My Seedlots" page, not to the "Seedlot Dashboard", can you confirm @SLDonnelly and @kevinginley ?

We will add the component in both my seedlot and seedlot dashboard page

@kevinginley
Copy link
Collaborator

@xiaopeng0202 @mgaseta

On my screen there are some finicky alignment issues (fyi I'm using Chrome)

image

image

Ideally the right edge of the "Go to seedlot" button would align with the right edge of the table, whereas here it is spilling outside of the table, which added a scrollbar to the bottom of my screen.

At the smallest breakpoint:
image

  • could you please add some padding between the "Go to seedlot" button and the field above it
  • and align the left edge of the "Go to seedlot" button with the left edge of the "Register a new seedlot"

As I'm playing around with the responsiveness, is there a way to consistently make the right edge of the "Register a new seedlot" button align with the right edge of the "Go to seedlot" button
image

Thank you!

Copy link
Contributor

@RMCampos RMCampos left a comment

Choose a reason for hiding this comment

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

Great job! Just a small suggestion for the regex and some styling issues :) 💯

frontend/src/utils/NumberUtils.ts Outdated Show resolved Hide resolved
@RMCampos
Copy link
Contributor

RMCampos commented Oct 1, 2024

Styling issues

  • My Seedlots

Can we improve this component on "My Seedlots" page? I can see it kind of sticking to the left menu, looking quite weird.
image

  • Fix title and subtitle according to page

I'm at the Seedlots page, and I still can see the review title and subtitle:

image

That's all =D great job!

@kevinginley
Copy link
Collaborator

The alignments are looking good, thank you!

Is there a way we could get consistent padding between the field and the "Go to seedlot button" between the different breakpoints?

Here we have quite a bit of space:
image

compared to:
image

But if it's going to be super tricky then I wouldn't worry about it.

@RMCampos
Copy link
Contributor

RMCampos commented Oct 3, 2024

@xiaopeng0202 nice. However I still can see "Review seedlot" at the Seedlots page.

Copy link
Contributor

@RMCampos RMCampos left a comment

Choose a reason for hiding this comment

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

Need to fix the title.

@RMCampos RMCampos self-requested a review October 3, 2024 17:31
Copy link
Contributor

@RMCampos RMCampos left a comment

Choose a reason for hiding this comment

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

All clear! Great job Xiao! 💯

@xiaopeng0202 xiaopeng0202 enabled auto-merge (squash) October 3, 2024 18:23
@xiaopeng0202 xiaopeng0202 merged commit d84c5e6 into main Oct 3, 2024
26 of 27 checks passed
@xiaopeng0202 xiaopeng0202 deleted the feat/1626-easier-seedlot-nv branch October 3, 2024 19:20
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.

Feature: easier navigation to seedlot registration from my seedlots page
4 participants