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.

Sign up for GitHub

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

Jump to bottom

[BUU] Left-align headers of new orders table #11634

Merged
merged 3 commits into from
Oct 13, 2023
Merged

[BUU] Left-align headers of new orders table #11634

merged 3 commits into from
Oct 13, 2023

Conversation

lauriejefferson
Copy link
Contributor

@lauriejefferson lauriejefferson commented Oct 6, 2023 •
edited by mkllnk
Loading

What? Why?

To correct the alignment of the table header and table rows on the /admin/orders screen in feature-preview mode.

What should we test?

  1. Visit admin/feature-toggle/features/admin_style_v3
  2. Select Fully Enable

As a Hub admin or Superadmin:
3. Visit the orders page - admin/orders
4. Notice the mismatch on the table header vs table contents orientation

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@mkllnk mkllnk added the technical changes only These pull requests do not contain user facing changes and are grouped in release notes label Oct 8, 2023
mkllnk
mkllnk approved these changes Oct 8, 2023
View reviewed changes
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

The code change looks good.

@mkllnk mkllnk changed the title changed table row alignment to align-left and removed padding from ta… [BUU] Left-align headers of new orders table Oct 8, 2023
@mariocarabotta mariocarabotta added pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-au staging.openfoodnetwork.org.au labels Oct 10, 2023
@mariocarabotta
Copy link
Collaborator

I tried to deploy this to staging AU using the staged-au tag, but it is not working. More details should be available in the action panel in Github

@RachL RachL added the pr-staged-fr staging.coopcircuits.fr label Oct 10, 2023
@RachL
Copy link
Contributor

RachL commented Oct 10, 2023 •
edited
Loading

Hello @lauriejefferson thanks for one more!

This is nice, I just have a question around the payment state column: the title seems to not be aligned with the column content, do you know why? Was the payment state column the only one centered?

image

@RachL RachL removed the pr-staged-fr staging.coopcircuits.fr label Oct 10, 2023
@mkllnk mkllnk added pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-au staging.openfoodnetwork.org.au labels Oct 11, 2023
@lauriejefferson
Copy link
Contributor Author

I added an nth-child selector to correct the alignment of the 'Payment State' heading. It's aligned according to the text-align property on the <th> parent element. CSS Grid would be the best choice to align the entire table using properties like grid-template-rows or subgrid to achieve proper alignment.

@dacook
Copy link
Member

dacook commented Oct 13, 2023

Thanks all. I took a look and noticed why the payment state was aligned wrong. There was broken code for showing a little coloured circle indicator on the payment state. Rather than try to explain it, I hope you don't mind that I went ahead and fixed, which now looks like this:
Screen Shot 2023-10-13 at 12 33 56 pm

@RachL can you please confirm this is a useful fix?

Also I couldn't see mention of reducing the padding on the grey title row, so @mariocarabotta could you confirm that's desired?

@dacook dacook force-pushed the 11610-left-align-all-table-content branch from 2004d1a to 2dc2406 Compare October 13, 2023 01:38
@RachL
Copy link
Contributor

RachL commented Oct 13, 2023

nice @dacook Yes this is useful, sorry for not noticing the missing circle 🤦‍♀️

@lauriejefferson
Copy link
Contributor Author

@dacook Thanks, didn't notice the missing circle either.

@drummer83 drummer83 self-assigned this Oct 13, 2023
@drummer83 drummer83 added the pr-staged-au staging.openfoodnetwork.org.au label Oct 13, 2023
lauriejefferson and others added 3 commits October 13, 2023 19:25
@lauriejefferson @drummer83
changed table row alignment to align-left and removed padding from ta…
2020a9f 
…ble header
@lauriejefferson @drummer83
removed empty spaces
e234b71
@dacook @drummer83
Fix payment state indicator
6e6854f 
Now the correct class is added, we can see the little coloured circles again.
@drummer83 drummer83 force-pushed the 11610-left-align-all-table-content branch from 2dc2406 to 6e6854f Compare October 13, 2023 17:25
@drummer83
Copy link
Contributor

Hi @lauriejefferson,
Thanks for another very nice contribution! 🙏

Before

image

After

image

Nice! Everything is now left aligned. 🎉
This is ready to go! Merging! 🚀

@drummer83 drummer83 merged commit 246fceb into openfoodfoundation:master Oct 13, 2023
49 of 50 checks passed
@drummer83 drummer83 removed the pr-staged-au staging.openfoodnetwork.org.au label Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkllnk mkllnk mkllnk approved these changes

Assignees

@drummer83 drummer83

@lauriejefferson lauriejefferson

Labels
technical changes only These pull requests do not contain user facing changes and are grouped in release notes
Projects
OFN Delivery board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUU, Orders page] Left align all table content
7 participants
@lauriejefferson @mariocarabotta @RachL @dacook @drummer83 @mkllnk @sigmundpetersen