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

Update features to latest mapping #22

Merged
merged 42 commits into from
May 28, 2024
Merged

Update features to latest mapping #22

merged 42 commits into from
May 28, 2024

Conversation

isabelle-dr
Copy link

@isabelle-dr isabelle-dr commented May 12, 2024

@Sergiodero I had a first go at updating the site with the latest mapping we landed on last week.
Feel free to contribute to this branch!

In this PR:

  • Make feature adjustments based on the latest mapping (see below)
  • Change "Pre-Requirement" to "Prerequisite", I think this term best serves what we're trying to transmit.
  • Place the prerequisites after the files & fields included table and on a single line when there is only one
  • Remove prerequisites from the Base group. I think we're clear on the site that the Base features are all needed to create a functioning dataset.
  • Smaller clean-up, editorial changes and simplifications of descriptions
  • Update the "Overview" page
  • Update the .yml file
Screenshot 2024-05-12 at 7 18 33 PM

@isabelle-dr
Copy link
Author

@Sergiodero this is ready for review!

@Sergiodero
Copy link

Thanks for this! I've just added a new commit editing the .yml file since the navigation wasn't updated. I took a quick look and the first thing that I'm noticing is that the rendering of the boxes for each feature in the overview page is a bit off (see image below).

image

I think a potential solution would be to update all descriptions to have between 60 and 70 characters (including spaces), that way the space would be more evenly distributed with two-line texts.

I'll continue with a more in-depth review tomorrow.

Copy link

@Sergiodero Sergiodero left a comment

Choose a reason for hiding this comment

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

I finalized my review and made some changes (three types):

  • Fixed the last card in the overview page which wasn't rendering properly
  • Fixed a lot of problematic links (my bad for not catching this earlier)
  • Fixed the order for a few Pre-requirements that were before the table of fields and files and not after

After this I only have two general comments:

  • I would keep formatting consistent across all pages by always listing prerequisites in bullet point form even if there is only one prerequisite associated with the feature.
  • I would add prerequisites back to the Base features in the same way that we previously did, using the text "Prerequisites: all other Base features"

These are small changes that I can take on if you agree with them.

On the other hand, I spend some time today finding ways to predefine the width of all cards in the overview page, but had no success, seems like there is no way to override the adjustment of the width to the length of the content. Considering this, could you help me revisiting the descriptions for all features in the overview page so that they all be around 70 characters? I think this is the most important change so lmk if you have capacity. Thanks!

@isabelle-dr
Copy link
Author

Thank you so much for all these fixes @Sergiodero!

I would keep formatting consistent across all pages by always listing prerequisites in bullet point form even if there is only one prerequisite associated with the feature.

Addressed in 7b0583b and cc3c770

I would add prerequisites back to the Base features in the same way that we previously did, using the text "Prerequisites: all other Base features"

Addressed in 5d9f85b

could you help me revisit the descriptions for all features in the overview page so that they all be around 70 characters?

I added spaces to the feature descriptions in 140cd1b so they are all 70 chars long. Is it better?

@Sergiodero
Copy link

Sergiodero commented May 24, 2024

Thanks @isabelle-dr! Fixes are looking good for the first two points, thanks for working on this!

Unfortunately the space quick-fix for the cards in the overview page didn't work (see image below), I can't think of any other workaround other than writing longer descriptions, we can use the previous ones which were a bit longer but I'm not sure how do you feel about that. Thoughts?

Screenshot 2024-05-24 at 9 32 28 AM

@isabelle-dr
Copy link
Author

@Sergiodero I've updated the descriptions.
Feel free to make changes directly until the rendering is right.

isabelle-dr and others added 2 commits May 27, 2024 15:02
Updates all descriptions to have 60-70 characters, harmonizing card width
Copy link

@Sergiodero Sergiodero left a comment

Choose a reason for hiding this comment

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

Thanks @isabelle-dr, I did some final adjustments and I think we're ready to merge!

Screenshot 2024-05-27 at 5 24 14 PM

@Sergiodero Sergiodero merged commit 4554102 into main May 28, 2024
@Sergiodero Sergiodero deleted the update-features branch May 28, 2024 14:44
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

2 participants