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

Improve metadata generation #5033

Merged
merged 4 commits into from
Jun 12, 2024
Merged

Improve metadata generation #5033

merged 4 commits into from
Jun 12, 2024

Conversation

joneff
Copy link
Contributor

@joneff joneff commented May 10, 2024

The code is self-explanatory, but here is anyway: values are processed and prettified in that order

  1. If the value is instance of SassMap, it's prettified as an object, with the values prettified recursively according to the rules hereby;
  2. If the value is instance of SassList, it's prettified as an array, with the items prettified according to the rules hereby;
  3. If the value is instance if SassString, it's prettified as string, with quotes removed where possible;
  4. If the value is instance of SassNumber, it's prettified as a number, if there are not units or string otherwise;
  5. If the value is instance of SassBoolean, it's prettified as boolean;
  6. If the value is equal to SassNull, it's prettified as null;
  7. In all other cases, the value is returned as string.

The prettified value is then stored as a new prettyValue field in variables.json, which is then copied to sassdoc-data.json.

@joneff joneff added Enhancement New feature of an existing functionality or an improvement of an existing functionality. Documentation Improvements or additions to documentation labels May 10, 2024
@joneff joneff requested a review from a team as a code owner May 10, 2024 16:16
@joneff joneff changed the title Add prettify values of sass variables for better display design system docs Add pretty values of sass variables for better display design system docs May 10, 2024
@joneff joneff self-assigned this May 10, 2024
@zhpenkov zhpenkov force-pushed the pretty-sass-values branch 3 times, most recently from 13a5218 to e3b856b Compare May 31, 2024 15:32
@zhpenkov
Copy link
Contributor

Corrections I have made to the original idea:

  1. SassList values prettified to a string. SassLists are typically presented as a string "10px 15px 0 0".
  2. Restrict adding prettyValue to only map variables.

@zhpenkov
Copy link
Contributor

zhpenkov commented Jun 5, 2024

I am using this PR to continue the efforts for the metadata improvements.
The changes I make are related to - https://github.com/telerik/kendo-themes-private/issues/240#issuecomment-2133557717

@zhpenkov zhpenkov changed the title Add pretty values of sass variables for better display design system docs Improve metadata generation Jun 5, 2024
@zhpenkov zhpenkov self-assigned this Jun 5, 2024
@zhpenkov zhpenkov requested review from a team and removed request for a team June 6, 2024 14:47
Copy link
Contributor

@epetrow epetrow left a comment

Choose a reason for hiding this comment

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

Looks good overall, just one question:

I noticed that we are displaying maps differently in the docs files now -- before the values were wrapped inside <code> while now they are inside a ul. Are we sure we want to remove the code tag?

packages/bootstrap/scss/core/color-system/_swatch.scss Outdated Show resolved Hide resolved
@zhpenkov
Copy link
Contributor

@epetrow The reason the <code> tag is replaced with <ul> <li> elements is because:

Before adding prettyValue for the map variables, we displayed the resolvedValue of each map, which is always resolved to string and is best visualized in an inline element - <code> tag.

(1: (0 2px 3px rgba(0, 0, 0, 0.04), 0 4px 16px rgba(0, 0, 0, 0.12)), 2: (0 4px 6px rgba(0, 0, 0, 0.06), 0 4px 16px rgba(0, 0, 0, 0.12)), 3: (0 6px 8px rgba(0, 0, 0, 0.08), 0 4px 16px rgba(0, 0, 0, 0.12)) ...

After adding the prettyValue, we can iterate over each key/value pair and display that information separately for better multiline data visualization - <ul> <li>.

  • 1: "0 2px 3px rgba(0, 0, 0, 0.04), 0 4px 16px rgba(0, 0, 0, 0.12)"
  • 2: "0 4px 6px rgba(0, 0, 0, 0.06), 0 4px 16px rgba(0, 0, 0, 0.12)"
  • 3: "0 6px 8px rgba(0, 0, 0, 0.08), 0 4px 16px rgba(0, 0, 0, 0.12)"

In terms of data tracking, there is no issue as both resolvedValue and prettyValue contain the same data. The only difference is that one is parsed with proper types, while the other is parsed as a string.

These changes to the .md doc files should be checked in the sites that directly use them to ensure everything works properly.

@zhpenkov zhpenkov requested a review from epetrow June 10, 2024 11:31
Copy link
Contributor

@epetrow epetrow left a comment

Choose a reason for hiding this comment

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

I noticed a few inconsistencies in how the data is displayed in the docs files now. It might be expected, however now we have:

Numbers being displayed as numbers - e.g. thin: 100

If a value has a unit it is a string - xxs: "0.5rem"

Also in some places there are escape characters that are being displayed in the docs now - serif: "\"Times New Roman\",

@zhpenkov zhpenkov added this to the 2024 Q3 (Aug) milestone Jun 12, 2024
@zhpenkov
Copy link
Contributor

@epetrow
I resolved the inconsistencies between strings / numbers and removed character escape symbols from the .md doc files

@zhpenkov zhpenkov requested a review from epetrow June 12, 2024 14:37
@zhpenkov zhpenkov merged commit 743978e into develop Jun 12, 2024
29 checks passed
@zhpenkov zhpenkov deleted the pretty-sass-values branch June 12, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation Enhancement New feature of an existing functionality or an improvement of an existing functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants