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

feat(Forms): Extension (beta) for simplified implementation of web forms through tailored functionality for layout and data handling #2420

Merged
merged 41 commits into from
Aug 17, 2023

Conversation

henit
Copy link
Contributor

@henit henit commented Jun 1, 2023

Summary

This is an extension to the base components of Eufemia for data input, display and layout surrounding data input/display.

Details

For more information about the architecture and idea behind how these components are structured, I would recommend reading the documentation for the extension that is added to the portal in the PR. Both in the base extension page (/uilib/extensions/deposit/), the provided demos there, as well as the base pages of each group of components (like /uilib/extensions/deposit/DataInput/) and each component page. For more detailed questions or discussions, please contact me on Slack.

Here are a few details in advance regarding some probable causes for discussions:

The name

The extension name "Deposit" was just something I came up with along the way, more of a working title. In the beginning, I simply called it "Data Input". This was as an alternative to some name including the word "Form" as that word has been the basis for a lot of different internal discussions, processes and libraries in DNB. To clariify the gist of this, these components is NOT a form library, the same way Eufemia is not a form library. Eufemia is a design system that has some components for data input, and these new components just add a layer of features on top of the existing data input components such as Input, Textarea and Checkbox to make it more plug & play for use in application features that includes input and static display of data from the user. When "Data Input" was the name, some paths to files ended up including "/data-input/data-input/data-input-string" and similar, so I wanted to call it something else, and something that did not crash with other commonly used terminology inside the used tech stacks, so I just called it "Deposit" as a metafor for people depositing money compared to inputing data to an application. Feel free to suggest alternative names for the package :-D

Also, the file structure (like DataInput/String.tsx for the component <DataInput.String />) was a choice to avoid too much repeated naming in paths. I can of course change this if needed.

The portal left menu

Since this package consists of a high number of components, it does fill the left menu in the portal up quite a bit. Unless this is not seen as a problem, maybe we should consider having menu items that can be expanded/collapsed, and some to be configured as collapsed by default so the extension does not grow the left menu so much?

Reusability

It is somewhat explained in the docs, but both input-components and value-components have reusable hooks to make it simple to make custom local components like these similar to the ones in the extension (that has a lot of the same features for internal state management etc without the need to make all that from scratch in every component. The same goes for wrapping components for those, like InputBlock and Value.

Spacing

One of the points of this library, is that as much as possible of the features should be "plug & play". So you put some fields on the scree, wrap them in a Card and/or Section components and add one or two MainHeading / SubHeading above parts of the features, and the spacing between each component should be in line with our design requirements by default in most cases, based on the order and composed structure of the components added to the view. Of course, there is hard to make something that is always perfect in this regard, and there is always need for custom cases / deviations. For that reason, you will find the defaults defined here and there as props inside components wrapping other components, but at the same time, all components should have standard spacing props (space, top, bottom, left and right) that can be overridden on every component you put on the screen.

FlexContainer

Many of the components in the extension wrap use of the provided components FlexContainer and FlexItem. These two components is exactly what the names says. Flexible components for building CSS-flexbox layouts, and with a few features on top (like stack). Many other layout components in the extension use these internally, and all the mentioned spacing details are handeled here. The naming of props here might be a bit confusing. These components have props named spacing which controls internal spacing (alternative for gap), while the name space is reused similar to other components in Eufemia for outer space around the element. If we can find a more describing name for the spacing-prop, maybe that is a good idea?

Also, the spacing in FlexContainer is handled via props instead of being defined in CSS, to ensure that the instructions for spacing given via props end up in the rendered base components from Eufemia (such as outer Div of components) and default to the standard spacing when no props are given.

Overridability and Context

You can read more about this in the docs, but the main idea here is that every component should be usable alone, and in combination with others. Like you should be able to put just one DataInput.String if you just need a text input with the added features of this component on top of Input or Textarea, but every property can be overridden. Then there is also the context provider (DataContext.Provider which provides common props that is distributed to input components inside the set of sub components, but props sent to each inner component always overrides outer props for composability.

Screenshot 2023-06-01 at 14 24 28

Test plan

@henit henit requested a review from tujoworker as a code owner June 1, 2023 12:30
@vercel
Copy link

vercel bot commented Jun 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
eufemia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 16, 2023 2:11pm

@langz
Copy link
Contributor

langz commented Jun 1, 2023

Great work @henit

It's happened a lot with main and v10 branches over the last months, so sorry for all the conflicts 😅
I think it will be easiest to try to cherrypick your commit onto main:
git fetch origin && git reset --hard origin/main && git cherry-pick c30ad5a2be5b43149ba06319aeaddcaaa6e67cd3

Where c30ad5a2be5b43149ba06319aeaddcaaa6e67cd3 is your commit hash from c30ad5a

And then resolve the conflicts, which I assume will be in some d.ts files, by accepting whatever(current change or incoming change), and then generate/update the .d.ts files again using the following command in your terminal: yarn test:types

@henit henit force-pushed the feat/data-input-extension branch from c30ad5a to 492ee6a Compare June 1, 2023 13:44
@henit
Copy link
Contributor Author

henit commented Jun 1, 2023

Thanks. And done ✅

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 6, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3992d54:

Sandbox Source
eufemia-starter Configuration

@henit
Copy link
Contributor Author

henit commented Jun 16, 2023

What about a name-discussion :-D Since this is packaged as a group of components in one extension and not a flat list of separate ones or similar, it could be good to name it something. Like I wrote in the description, i called it "Deposit" as a working title to avoid mixups with other terminology (like the word "form" which has kind of a strange mixed meaning internally in the organization). Another suggestion could be something like "Data IO" since it both inputs and displays (with the value components) data to the user and has features to distribute and collect/merge data to/from source objects..
Any thoughts on naming @tujoworker / @langz ?

@tujoworker
Copy link
Member

I suggest we use Forms. Such a set of components needs a name that is easy understandable and communicative. It describes the overall meaning and what it is intended to be used for.

I could not find any good synonyms either.

@langz
Copy link
Contributor

langz commented Aug 15, 2023

Are there any components/compositions of this PR where we would consider adding screenshot tests?
On the lowest "component level", we already have screenshot tests for like Buttons, etc.
But is there a composition of several components in a certain way or something as part of this forms library, that we could want to have screenshot tests for?

If so, I could assist with adding them :)
Thoughts @tujoworker and @henit?

@langz
Copy link
Contributor

langz commented Aug 15, 2023

I see we've added quite a few data-testid properties as part of this PR.
In v10 we removed a lot of data-testid from our codebase, as we felt that it wasn't really the "best way" of accessing elements for testing, and that it exists better ways that adds more values to the tests. We also felt that we just had a lot of data-testid properties in our code which was only for testing, and just generating more code.

We don't have to remove this as part of this PR, as I assume/guess that it's already used by other apps, but I feel like we could/should remove it sometime in the future to "stay true" to that "we don't use data-testid's unless it's our only option" 😂

It might be the correct option to use data-testid's here though, any thoughts? 🤔 🧠

@langz
Copy link
Contributor

langz commented Aug 15, 2023

Just a small observation, not sure if it's the "demo code" for the case demo or if it's an error inside a component or so, but I can't seem to select "Etableringsland og sånt som det". Seems to happen after I fill in most of the other fields:

Screen.Recording.2023-08-15.at.13.35.48.mov

I think/guess that it's just something to do with the example code, meaning it's probably not very important to fix.

@henit
Copy link
Contributor Author

henit commented Aug 15, 2023

Are there any components/compositions of this PR where we would consider adding screenshot tests? On the lowest "component level", we already have screenshot tests for like Buttons, etc. But is there a composition of several components in a certain way or something as part of this forms library, that we could want to have screenshot tests for?

If so, I could assist with adding them :) Thoughts @tujoworker and @henit?

There is the Field.PostalCodeAndCity component, which renders two components inside, targetting separate values in the target data source. Field.PhoneNumber also render two fields, but it is for parts of one value (country code and the rest of the number).

Copy link
Member

@tujoworker tujoworker left a comment

Choose a reason for hiding this comment

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

Fantastic work 👏

@henit henit merged commit 74700c0 into main Aug 17, 2023
@henit henit deleted the feat/data-input-extension branch August 17, 2023 08:45
tujoworker pushed a commit that referenced this pull request Aug 17, 2023
## [10.6.0](v10.5.0...v10.6.0) (2023-08-17)

### 📝 Documentation

* add "Messageboxes" to FormStatus for the equivalent Figma name ([#2560](#2560)) ([bf6a19f](bf6a19f))
* add docs on how to link to a specific theme ([#2555](#2555)) ([54b9f1b](54b9f1b)), closes [#2374](#2374)
* add theming link to the contribution guide ([#2558](#2558)) ([e88bd72](e88bd72))

### ✨ Features

* **Anchor:** inline style sbanken ([#2550](#2550)) ([3511f09](3511f09))
* **Button:** sbanken hover and error ([#2523](#2523)) ([3ea2850](3ea2850))
* **Forms:** Extension (beta) for simplified implementation of web forms through tailored functionality for layout and data handling ([#2420](#2420)) ([74700c0](74700c0))
* **Input:** sbanken styling ([#2540](#2540)) ([db36f09](db36f09))
* **Textarea:** sbanken theme ([#2551](#2551)) ([d8cfb1c](d8cfb1c))

### 🐛 Bug Fixes

* **Forms:** fix circular imports issue ([#2566](#2566)) ([3ac4276](3ac4276))
* **NumberFormat:** accept options like maximumFractionDigits ([#2557](#2557)) ([2e09e80](2e09e80))
@tujoworker
Copy link
Member

🎉 This PR is included in version 10.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants