[OSCI] Clean/breadcrumb updates - Refactoring use of 'let' for 'const' keyword and CSS modifier '--last' for one class only #1144

BigSamu · 2023-11-05T22:56:47Z

Description

This PR addresses the new requests asked for the OuiBreadcrumb component as detailed below:

The specific updates done were:

Refactoring code in breadcrumbs.tsx file using const keywords instead of let.
Refactoring code in _breadcrumbs.scss file removing the CSS--last modifier from .ouiBreadcrumb class and keeping it only in .ouiBreadcrumbWrapperclass.

Issues Resolved

Issue #377

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: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

…eadcrumb class, keeping it only in .ouiBreadcrumbWrapper class Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

BSFishy

Looks good, except this example breaks:

Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

BigSamu · 2023-11-08T12:07:04Z

Looks good, except this example breaks:

@BSFishy issue resolved. It was a minor mistake in the definition for calculating the variable calculatedMax. Thanks for the head-ups.

BigSamu · 2023-11-17T11:58:48Z

@BSFishy,

Any updates on this side? The corrections you requested are ready for review.

Regards,

Samuel

BSFishy

Still need to test locally, but other than this change it looks good

src/components/breadcrumbs/breadcrumbs.tsx

BSFishy · 2023-12-04T18:44:53Z

Visually looks good. Want to see this change, though, for cleanliness reasons

…ter readability of code Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

BSFishy

Only visual change is that the collapsed breadcrumb ellipses are a slightly brighter color, which I think was the original intention. Looks good!

BigSamu · 2023-12-05T18:51:19Z

Only visual change is that the collapsed breadcrumb ellipses are a slightly brighter color, which I think was the original intention. Looks good!

Hi @BSFishy, do you want me to review the color you mentioned for the breadcrumb ellipsis?

BSFishy · 2023-12-05T18:58:18Z

Hi @BSFishy, do you want me to review the color you mentioned for the breadcrumb ellipsis?

Nope, the color looks better imo. I think I was trying to make this change when I redid this implementation but was running into issues and didn't want to hold it for longer than I already was, so this change is good in my book!

Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

BigSamu · 2023-12-05T19:04:22Z

Hi @BSFishy, do you want me to review the color you mentioned for the breadcrumb ellipsis?

Nope, the color looks better imo. I think I was trying to make this change when I redid this implementation but was running into issues and didn't want to hold it for longer than I already was, so this change is good in my book!

Great! Btw just solved a merge conflict in the CHANGELOG.md that popped up here in the GitHub Interface. All good now.

Signed-off-by: Josh Romero <rmerqg@amazon.com>

BigSamu · 2024-01-01T16:32:43Z

@BSFishy, @joshuarrrr

PR updated with the main branch. Are we good to go to merge?

BSFishy · 2024-01-02T18:32:16Z

PR updated with the main branch. Are we good to go to merge?

Just need another reviewer

…' keyword and CSS modifier '--last' for one class only (#1144) * refactoring changing 'let' variables with 'const' Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * refactoring changing 'let' variables with 'const' Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * refactoring _breadcrumbs.sccs, removing '--last' modifier from .ouiBreadcrumb class, keeping it only in .ouiBreadcrumbWrapper class Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * refactoring _breadcrumbs.sccs, removing '--last' modifier from .ouiBreadcrumb class, keeping it only in .ouiBreadcrumbWrapper class Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * updating CHANGELOG.md file Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * fixing calculation of calculatedMax, breadcrumbs.tsx, line 310 Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> * updating breadcrumbs JSX according suggestion from maintainer for better readability of code Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> --------- Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> Signed-off-by: Josh Romero <rmerqg@amazon.com> Co-authored-by: Josh Romero <rmerqg@amazon.com> (cherry picked from commit 064a02c) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…' keyword and CSS modifier '--last' for one class only (#1144) (#1248) * refactoring changing 'let' variables with 'const' * refactoring changing 'let' variables with 'const' * refactoring _breadcrumbs.sccs, removing '--last' modifier from .ouiBreadcrumb class, keeping it only in .ouiBreadcrumbWrapper class * refactoring _breadcrumbs.sccs, removing '--last' modifier from .ouiBreadcrumb class, keeping it only in .ouiBreadcrumbWrapper class * updating CHANGELOG.md file * fixing calculation of calculatedMax, breadcrumbs.tsx, line 310 * updating breadcrumbs JSX according suggestion from maintainer for better readability of code --------- (cherry picked from commit 064a02c) Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com> Signed-off-by: Josh Romero <rmerqg@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Josh Romero <rmerqg@amazon.com>

BigSamu added 3 commits November 5, 2023 20:55

refactoring changing 'let' variables with 'const'

422df63

Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

refactoring changing 'let' variables with 'const'

a5ac107

Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

refactoring _breadcrumbs.sccs, removing '--last' modifier from .ouiBr…

4cb6024

…eadcrumb class, keeping it only in .ouiBreadcrumbWrapper class Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

BigSamu requested review from seanneumann, AMoo-Miki, rednaksi91, KrooshalUX, BSFishy, bandinib-amzn, ashwin-pc, joshuarrrr, andreymyssak and SergeyMyssak as code owners November 5, 2023 22:56

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

BigSamu mentioned this pull request Nov 5, 2023

[Code cleanup] Clean up new code for breadcrumb updates #377

Open

BigSamu added 2 commits November 5, 2023 23:00

refactoring _breadcrumbs.sccs, removing '--last' modifier from .ouiBr…

367df8d

…eadcrumb class, keeping it only in .ouiBreadcrumbWrapper class Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

updating CHANGELOG.md file

ce9333e

Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

BSFishy added the OSCI label Nov 6, 2023

BSFishy reviewed Nov 7, 2023

View reviewed changes

BigSamu added 2 commits November 8, 2023 11:40

Merge branch 'main' into clean/breadcrumb-updates

43910a6

fixing calculation of calculatedMax, breadcrumbs.tsx, line 310

9c0bcf0

Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

BSFishy reviewed Dec 4, 2023

View reviewed changes

src/components/breadcrumbs/breadcrumbs.tsx Outdated Show resolved Hide resolved

updating breadcrumbs JSX according suggestion from maintainer for bet…

5ad83b3

…ter readability of code Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

BSFishy approved these changes Dec 5, 2023

View reviewed changes

Merge branch 'main' into clean/breadcrumb-updates

c648e75

Signed-off-by: Samuel Valdes Gutierrez <valdesgutierrez@gmail.com>

joshuarrrr and others added 2 commits December 11, 2023 17:59

Merge branch 'main' into clean/breadcrumb-updates

4122946

Signed-off-by: Josh Romero <rmerqg@amazon.com>

Merge branch 'main' into clean/breadcrumb-updates

40a94f1

ashwin-pc approved these changes Jan 11, 2024

View reviewed changes

ashwin-pc merged commit 064a02c into opensearch-project:main Jan 11, 2024
14 checks passed

BigSamu deleted the clean/breadcrumb-updates branch January 11, 2024 20:00

AMoo-Miki added backport 1.x backport 1.5 labels Feb 8, 2024

opensearch-trigger-bot bot mentioned this pull request Feb 8, 2024

[Backport 1.x] [OSCI] Clean/breadcrumb updates - Refactoring use of 'let' for 'const' keyword and CSS modifier '--last' for one class only #1248

Merged

opensearch-trigger-bot bot mentioned this pull request Feb 8, 2024

[Backport 1.5] [OSCI] Clean/breadcrumb updates - Refactoring use of 'let' for 'const' keyword and CSS modifier '--last' for one class only #1249

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[OSCI] Clean/breadcrumb updates - Refactoring use of 'let' for 'const' keyword and CSS modifier '--last' for one class only #1144

[OSCI] Clean/breadcrumb updates - Refactoring use of 'let' for 'const' keyword and CSS modifier '--last' for one class only #1144

BigSamu commented Nov 5, 2023 •

edited

Loading

BSFishy left a comment

BigSamu commented Nov 8, 2023

BigSamu commented Nov 17, 2023

BSFishy left a comment

BSFishy commented Dec 4, 2023

BSFishy left a comment

BigSamu commented Dec 5, 2023

BSFishy commented Dec 5, 2023

BigSamu commented Dec 5, 2023

BigSamu commented Jan 1, 2024 •

edited

Loading

BSFishy commented Jan 2, 2024

[OSCI] Clean/breadcrumb updates - Refactoring use of 'let' for 'const' keyword and CSS modifier '--last' for one class only #1144

[OSCI] Clean/breadcrumb updates - Refactoring use of 'let' for 'const' keyword and CSS modifier '--last' for one class only #1144

Conversation

BigSamu commented Nov 5, 2023 • edited Loading

Description

Issues Resolved

Check List

BSFishy left a comment

Choose a reason for hiding this comment

BigSamu commented Nov 8, 2023

BigSamu commented Nov 17, 2023

BSFishy left a comment

Choose a reason for hiding this comment

BSFishy commented Dec 4, 2023

BSFishy left a comment

Choose a reason for hiding this comment

BigSamu commented Dec 5, 2023

BSFishy commented Dec 5, 2023

BigSamu commented Dec 5, 2023

BigSamu commented Jan 1, 2024 • edited Loading

BSFishy commented Jan 2, 2024

BigSamu commented Nov 5, 2023 •

edited

Loading

BigSamu commented Jan 1, 2024 •

edited

Loading