Optimize calc() usage in scss files #1158

ShatilKhan · 2023-11-16T15:07:41Z

Description

Optimization of some math equations inside calc() in scss files was done.

Issues Resolved

Check List

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Nuraiym Omurbekova <[email protected]> Co-authored-by: Josh Romero <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Also: * Implement sass variable extraction * Implement sass variable importing in typescript * Add guards to post-install cleanup * Make sources compatible with dart sass by wrapping divisions in `calc()` and adding units were necessary * Fold in `@elastic/charts/dist/theme.scss` to modify it for dart-sass compatibility * Break out compiling `charts` Signed-off-by: Miki <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

* typescript bump: 4.1.6 Signed-off-by: Danila Gulderov <[email protected]> * typescript bump: 4.2.4 Signed-off-by: Danila Gulderov <[email protected]> * typescript bump: 4.3.5 Signed-off-by: Danila Gulderov <[email protected]> * typescript bump: 4.4.4 Signed-off-by: Danila Gulderov <[email protected]> * typescript bump: 4.5.5 Signed-off-by: Danila Gulderov <[email protected]> * typescript bump: 4.6.4 Signed-off-by: Danila Gulderov <[email protected]> * CHANGELOG.md updated Signed-off-by: Danila Gulderov <[email protected]> * bump typescript-eslint/eslint-plugin: 5.62.0 bump typescript-eslint/parser: 5.62.0 node 16 is required for typescript-eslint: 6.0.0 Signed-off-by: Danila Gulderov <[email protected]> * CHANGELOG.md update Signed-off-by: Danila Gulderov <[email protected]> * move typescript update to regular section Signed-off-by: Danila Gulderov <[email protected]> * changelog message improvement Signed-off-by: Danila Gulderov <[email protected]> * fix typings Signed-off-by: Danila Gulderov <[email protected]> * cleanup useUnknownInCatchVariables Signed-off-by: Danila Gulderov <[email protected]> * logical or => nullish coalescing Signed-off-by: Danila Gulderov <[email protected]> * wrap with String Signed-off-by: Danila Gulderov <[email protected]> * Update CHANGELOG.md Move CHANGELOG entry to the unreleased section Signed-off-by: Miki <[email protected]> * Add back `onPinClick` check in `OuiPinnableListGroup` Update pinnable_list_group.tsx Signed-off-by: Miki <[email protected]> * Enhance error message extraction in `validateFieldValue` Update default_syntax.ts Signed-off-by: Miki <[email protected]> * Remove e.code from default_syntax.ts Update default_syntax.ts Signed-off-by: Miki <[email protected]> * Lint default_syntax.ts Signed-off-by: Miki <[email protected]> --------- Signed-off-by: Danila Gulderov <[email protected]> Signed-off-by: Matt Provost <[email protected]> Signed-off-by: Miki <[email protected]> Co-authored-by: Matt Provost <[email protected]> Co-authored-by: Josh Romero <[email protected]> Co-authored-by: Miki <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

On line 88, in order to optimize We perform the subtraction within the map-get function directly. This simplifies the calculation by removing the unnecessary parentheses around map-get($map, 'percentage'). Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

ShatilKhan · 2023-11-17T10:52:28Z

These same kind of conflicts keep appeared in my previous PR
I'm curious about why this kind of conflict actually occurs?

<<<<<<< calc
=======
>>>>>>> main

What does this actually mean?

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Signed-off-by: ShatilKhan <[email protected]>

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

ShatilKhan · 2023-11-17T11:01:01Z

I'm just gonna make a new PR, a lot of problem with this one

joshuarrrr · 2023-11-17T16:28:12Z

These same kind of conflicts keep appeared in my previous PR I'm curious about why this kind of conflict actually occurs?
<<<<<<< calc
=======
>>>>>>> main
What does this actually mean?

@ShatilKhan Those lines indicate a merge conflict, where git is unsure whether which branch has the "correct" or desired changes. So it keeps both and wraps them in those markers so that you can manually choose (or replace with a combination). The first section is the line(s) as it appears in the calc branch and the second is the line as it appears in the main branch. See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/about-merge-conflicts and https://code.visualstudio.com/docs/sourcecontrol/overview#_merge-conflicts

ShatilKhan · 2023-12-11T05:15:22Z

Thanks for the explanation @joshuarrrr

Signed-off-by: Shahriar Shatil <[email protected]>

ShatilKhan · 2023-12-11T05:51:23Z

I resolved all the conflicts
This one is ready for review

BSFishy · 2023-12-12T16:53:55Z

It looks like the linter is throwing errors: https://github.com/opensearch-project/oui/actions/runs/7163472679/job/19525060011?pr=1158#step:5:17

Also looks like build got broken: https://github.com/opensearch-project/oui/actions/runs/7163472679/job/19525059465?pr=1158#step:5:123

Signed-off-by: Shahriar Shatil <[email protected]>

ShatilKhan

fixed linting failures

Signed-off-by: Shahriar Shatil <[email protected]>

AMoo-Miki

A lot of the changes are unnecessary and in fact, detrimental to readability and maintainability.

AMoo-Miki · 2024-09-09T16:36:57Z

scripts/compile-themes.js

+const util = require('util')
+const { writeFile, mkdir } = require('fs/promises'


What happened here? Please revert.

AMoo-Miki · 2024-09-09T16:37:34Z

scripts/babel/variables-from-scss/index.js

@@ -64,4 +65,5 @@ module.exports = (babel) => ({
      }
    },
  },
+


Please remove unnecessary space.

AMoo-Miki · 2024-09-09T16:37:39Z

scripts/compile-charts.js

@@ -28,6 +28,7 @@
 * under the License.
 */

+


Please remove unnecessary space.

AMoo-Miki · 2024-09-09T16:38:05Z

scripts/compile-themes.js

+  await Promise.all
+    inputFilenames.map(async (inputFilename) => 


Please revert.

AMoo-Miki · 2024-09-09T16:41:24Z

scripts/compile-themes.js

+          chalk`{green ✔} Finished compiling {gray ${inputFilename}} to ${outputFilename
+            .map((filename) => chalk.gray(filename)


Please revert

ShatilKhan requested review from seanneumann, AMoo-Miki, rednaksi91, KrooshalUX, BSFishy, bandinib-amzn, ashwin-pc, joshuarrrr, andreymyssak and SergeyMyssak as code owners November 16, 2023 15:07

github-actions bot added the first-time-contributor label Nov 16, 2023

Naurabay and others added 7 commits November 16, 2023 21:10

Update README.md (opensearch-project#570)

1dcc096

Signed-off-by: Nuraiym Omurbekova <[email protected]> Co-authored-by: Josh Romero <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Update _card.scss

2059084

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Update _combo_box.scss

24b3d37

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Update _control_bar.scss

35d11f2

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

ShatilKhan force-pushed the calc branch from a21ec7d to c452627 Compare November 17, 2023 10:43

ShatilKhan and others added 7 commits November 17, 2023 16:58

Update _tool_tip.scss

c744b56

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Update _range_thumb.scss

b9d2042

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Update tree_view.scss

2a23689

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Update range_thumb.scss

7d3c3f5

Signed-off-by: ShatilKhan <[email protected]>

Update Changelog

9814449

Signed-off-by: ShatilKhan <[email protected]>

Update _range_slider.scss

2aee718

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

Update _resizable_button.scss

c387876

Signed-off-by: Shahriar Shatil <[email protected]> Signed-off-by: ShatilKhan <[email protected]>

ShatilKhan force-pushed the calc branch from c452627 to c387876 Compare November 17, 2023 10:58

ShatilKhan closed this Nov 17, 2023

ShatilKhan reopened this Dec 11, 2023

Merge branch 'main' into calc

9cc2a25

Signed-off-by: Shahriar Shatil <[email protected]>

Update index.js

c69648e

Signed-off-by: Shahriar Shatil <[email protected]>

ShatilKhan commented Jan 5, 2024

View reviewed changes

Merge branch 'main' into calc

0f5542f

Signed-off-by: Shahriar Shatil <[email protected]>

AMoo-Miki requested changes Sep 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize calc() usage in scss files #1158

Optimize calc() usage in scss files #1158

ShatilKhan commented Nov 16, 2023

ShatilKhan commented Nov 17, 2023

ShatilKhan commented Nov 17, 2023

joshuarrrr commented Nov 17, 2023

ShatilKhan commented Dec 11, 2023

ShatilKhan commented Dec 11, 2023

BSFishy commented Dec 12, 2023

ShatilKhan left a comment

AMoo-Miki left a comment

AMoo-Miki Sep 9, 2024

AMoo-Miki Sep 9, 2024

AMoo-Miki Sep 9, 2024

AMoo-Miki Sep 9, 2024

AMoo-Miki Sep 9, 2024

		const util = require('util')
		const { writeFile, mkdir } = require('fs/promises'

@@ @@ -64,4 +65,5 @@ module.exports = (babel) => ({ @@
                     }
                   },
                 },

		await Promise.all
		inputFilenames.map(async (inputFilename) =>

		chalk`{green ✔} Finished compiling {gray ${inputFilename}} to ${outputFilename
		.map((filename) => chalk.gray(filename)

Optimize calc() usage in scss files #1158

Are you sure you want to change the base?

Optimize calc() usage in scss files #1158

Conversation

ShatilKhan commented Nov 16, 2023

Description

Issues Resolved

Check List

ShatilKhan commented Nov 17, 2023

ShatilKhan commented Nov 17, 2023

joshuarrrr commented Nov 17, 2023

ShatilKhan commented Dec 11, 2023

ShatilKhan commented Dec 11, 2023

BSFishy commented Dec 12, 2023

ShatilKhan left a comment

Choose a reason for hiding this comment

AMoo-Miki left a comment

Choose a reason for hiding this comment

AMoo-Miki Sep 9, 2024

Choose a reason for hiding this comment

AMoo-Miki Sep 9, 2024

Choose a reason for hiding this comment

AMoo-Miki Sep 9, 2024

Choose a reason for hiding this comment

AMoo-Miki Sep 9, 2024

Choose a reason for hiding this comment

AMoo-Miki Sep 9, 2024

Choose a reason for hiding this comment