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

Add Compressed form components, color-picker, and combobox, and small button components and filter-button #1301

Merged

Conversation

AMoo-Miki
Copy link
Collaborator

@AMoo-Miki AMoo-Miki commented Jul 12, 2024

Description

This PR introduces special variants of the necessary components with fixed size or density properties, in order to mitigate potential complexities and conflicts that could arise from attempting to revert size and density modifications directly within the original components, especially as the codebase evolves over time.

Long version

Modifications to form elements and buttons without explicit sizing attributes are being implemented in OSD to optimize space utilization and enhance the overall aesthetic appeal. These adjustments involve setting size="s" or compressed={true} on the elements. However, a comprehensive review of OUI's typography and sizing conventions is scheduled, which could potentially necessitate the reversal of these modifications.

As time progresses, reversal of the thousands of size and density modifications will become increasingly intricate due to the accumulation of neighboring code resulting in conflicts that must be addressed. Furthermore, the pre-existing sizing and density properties will become indistinguishable from the newly implemented modifications, compounding the complexity of the reversal task.

This PR introduces special variants of the necessary components with fixed size or density properties. OSD and plugins will use these variants which are marked @internal to indicate that they are not meant for public consumption. This strategy ensures that if a decision is made to revert to the previous sizing and density specifications, the process of identifying and modifying the components will be more straightforward. Instead of having to search through and update individual size and density properties scattered across numerous instances of the original components, the instances of the special variants can be located and replaced more efficiently. If the typography revisit does not necessitate a reversal, transitioning the special variants to utilize the original components with the desired size and density properties would be a straightforward process as well.

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • All tests pass
    • yarn lint
    • yarn test-unit
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

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.

@ashwin-pc
Copy link
Member

Im not getting the full picture here. It looks like we are introducing new compressed components here but i dont see any aliasing. Also if this is a temporary change, can we mark them as experimental so that we can remove them without a breaking change?

@AMoo-Miki AMoo-Miki force-pushed the compressed-rows-small-buttons branch from 28453c9 to f0c44e8 Compare July 15, 2024 15:55
@AMoo-Miki
Copy link
Collaborator Author

AMoo-Miki commented Jul 15, 2024

Im not getting the full picture here. It looks like we are introducing new compressed components here ...

The new components would ease the task of locating and reworking the thousands of changes that are going into OSD and plugins for the upcoming typography changes.

... but i dont see any aliasing.

Aliases in OUI are created at build-time.
PS, changed the commit messages and changelog to avoid confusion with the word alias.

Also if this is a temporary change, can we mark them as experimental so that we can remove them without a breaking change?

Yes! Added @internal.

@virajsanghvi
Copy link
Collaborator

Just to validate, the long term goal would be for these variants not to exist and just to use existing component + prop, right?

@AMoo-Miki
Copy link
Collaborator Author

Just to validate, the long term goal would be for these variants not to exist and just to use existing component + prop, right?

Yes.

@AMoo-Miki AMoo-Miki merged commit 7a6a456 into opensearch-project:main Jul 17, 2024
17 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 17, 2024
… button components and filter-button (#1301)

* Add internal small button family components

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

* Add internal compressed form family components

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

* Add internal compressed color picker component

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

* Add internal compressed combo-box component

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

* Add internal compressed filter-button component

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

* Update changelog

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

---------

Signed-off-by: Miki <[email protected]>
(cherry picked from commit 7a6a456)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
AMoo-Miki pushed a commit that referenced this pull request Jul 17, 2024
… button components and filter-button (#1301) (#1302)

* Add internal small button family components

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

* Add internal compressed form family components

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

* Add internal compressed color picker component

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

* Add internal compressed combo-box component

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

* Add internal compressed filter-button component

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

* Update changelog

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

---------

Signed-off-by: Miki <[email protected]>
(cherry picked from commit 7a6a456)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants