Conversation
|
Given the following is correct:
I think most importantly, before step 2, we should discuss in a (series of?) meeting(s). Especially about the "What belongs where". I'm afraid that a lot of components, which are technically molecules might go into If we decide against putting anything into Where should Does widgets then turn into a collection molecules, organisms, translated atoms, translated molecules/organisms, dhis2-related atoms (if that even exists), dhis2-data-related components, dhis2-api-related components? That would defeat the very purpose of having different libraries imo. |
Quite the contrary, there will be a lot of atoms in core. They will be small, and only contain styles and markup. The core components will then be composed in widgets to a form that users can utilize in their applications.
It does make sense, and I would challenge the parenthesis that seems to be the main drawback you mention about it adding a maintenance burden. It lowers the burden since CSS and markup should very rarely have to change. And if the atoms rarely have reason to change, they become much more stable in terms of reuse, which allows us to use those components in many other components and contexts they weren't designed for. This would in effect lower the maintenance burden overall.
One way to think of it is as DHIS2-related atoms would have to be split up into "pure" atoms, and then they go in core. I'm not sure about the difference between these: dhis2-data-related components, dhis2-api-related components. If it doesn't make sense to keep these in widgets (because they are their own class of components), we can easily add another package to the monorepo without breaking or introducing anything new for our users, save for additional imports.
Good eye. I left them hanging around while refactoring as they don't seem to quite "fit in" as you say. Perhaps we need a utils package as well where utility components such as these can live. |
So we want to go 100% the atomic design way and create our molecules from nothing but atoms? Which basically means that there shouldn't be a single native html element used in Either way, this means that we'll have an increased initial amount of work, either because of out-sourcing the atoms from our currently "atomic" molecules or by creating a proper layout library and understand the motivation but question the benefit it might give us. From a pragmatic perspective this seems overkill to me. Molecules like the We won't split existing Molecules and organisms until it's beneficial, which also embraces the "Easy to add stuff to widgets, hard to add stuff to core" philosophy
I would argue that the styles of our molecules are rarely subject to change as that's driven by the design system rather than a developer's preference / app's need. Pointing back here to:
That's clear to me, what I meant is: The rules for what to add to That might add complexity in our structure once we have components in multiple packages (let's say a The way I see it is that the only really good reason for a monorepo is that we now can separate the structure into SRP-following structures while the I'm not trying to say that ☝️ is the correct structural abstraction, but I just want to demonstrate that there are other possible solutions that we might benefit from. |
This is true. I see it more as in a literal graphical widget[0], but the downside is that the core components are also graphical widgets. To reconcile that, the distinction I draw between them is to lean on atoms and molecules. Atoms (core components) are not supposed to be used "as is" in a UI. They should be composed in some way to form a functional component, a molecule. The former goes in core and the latter in widgets. I'm open to removing ui-widgets and introducing multiple libraries in its place. With the monorepo, we can do this behind the scenes when we have a clear idea of what we need here. It can be transparent to users, so it's not necessarily a breaking change. So I think that moving forward with what we know we need now, and having the option to change e.g. widgets later, is a pragmatic way forward. |
No, I don't think we need to go 100%. The |
|
Discussed this a little bit in Slack here To paraphrase I think I agree with @Mohammer5 here. DHIS2-specific components with Data and Translation requirements should not live in the same package as
Obviously this is just my imperfectly-informed opinion, but I the structure is clear, it respects the goals @varl has for differentiating between "pure" atoms and basic "graphical widgets" (living in |
I agree, and this is what I did. I see that you introduced a new package (ui-base) and what I did was empty out ui-core to make room for the base components. |
|
@ismay and @Mohammer5 have repeatedly argued that "what goes where" in terms of core and widgets is confusing, so the question is: Would the base, core, and widgets, split resolve those concerns? This split still means that as soon as any component needs translations; then it becomes a widget. |
|
We're all after the same thing, just going about it in different way :-D I think it’s worth keeping the APIs of The most important thing for me, though, and maybe @Mohammer5 as well, is that I think the dependencies of DHIS2 Organisms will be fundamentally different from those of the basic molecules, so they shouldn’t live in the same package - the dependency boundary is the only real benefit of splitting the monorepo into sub-packages, and I think it's a powerful tool for internal separation of concerns. |
|
The translation question is a good one - which are the core components that might require translations? |
Indeed. I didn't consider a new "base" package as I was stuck in my perception that "core" is the base. Maybe that's enough to lay the "what goes where" question to rest?
The MultiSelect was discussed having a default string for e.g. the loading state of the dropdown. It has come up several times though, but we've asked @cooper-joe to rethink the design specs and not force a default text into the component. Practically speaking, the base components and even the core component could be untranslated, but e.g. the MultiSelectField component could live in widgets and handle the translation on that level. |
|
Got it. If there end up being a lot of translations-required molecules we could introduce one more level ( |
|
Yeah we have a lot more options in a monorepo to introduce levels as they are needed. |
I saw the mention and discussion on slack. I'll respond here, I hope I didn't misinterpret anything (let me know if I have). So I'm assuming that we're talking about @amcgee's suggestion?
And I assume we're still inspired by / adhering to Brad Frost's Atomic Design guidelines? First off, I'd be fine with whatever abstraction we end up going with. As long as it's something I can learn to use I'm fine with it 👍. To explain why I found the organisation confusing sometimes: I think the problems I've had with our categorisations in the past were because some of our categories to me do not clearly convey their purpose from their name. I do find But If possible, I'd prefer category names that make it completely clear what their purpose is. Just so that we don't discuss as much about it. But in the end I'm fine with whatever we settle on. |
I agree. I am struggling to come up with very sharp names for "base", "core", and "widget" categories though. |
re: namingI'd like to contribute some thoughts to the library names.
|
|
The above names are really well thought out, and my suggestion of this is not a reflection on that, but an alternative could be to just put them all in a single category (base, core and widgets -> single group)? I feel like we're trying to use categories to enforce proper composability, component reuse, etc. But maybe naming is just a tricky tool when trying to enforce those things. You're either ending up with a system that defines hierarchical levels like atomic design, or something related. Which always has to be explained, because the category names don't follow as directly from what they contain. As opposed to forms, constants, icons, etc. which are kind of self evident. |
The An application is preferably going to depend on They still have the option to do so, if need be.
The "et cetera" in that list contains the non-technical reasons to split up the packages: dependency management, API consistency and stability, and ease of peer reviews. To name a few (I was so close to add "etc." to that list too 😄). If a component is added my goal is to have basically a checklist for what to look for when doing a review: For base:
For core:
For widgets:
This checklist is obviously a draft just pulled from the coffee-addled recesses of my mind, however, such a list is easier to work out and provide context around the code structure and organization when there are interfaces between architectural layers. More importantly, a checklist would allow for new UI-developers to do an initial self-review and get an idea of if their implementation is inline with the structure and way we build components. It wouldn't be perfect, but we could guide devs into the style of UI without having to hand-train everyone that wants to contribute. We could have a checklist anyway, it's not coupled to this discussion in any way, but it has been difficult to write one that also explains what goes where, historically speaking. |
Ah nice, that checklist helps with understanding your goals for the categories. Let me get properly caffeinated first and then I'll take a look and see if I can help with the naming. |
|
How about:
I know they're ugly names, but do they convey what's supposed to go in each category to you guys? To me it does provide clear distinction just from the names. Is it a base for other components to build on: |
|
Before we continue talking about this, I think there are several issues that are being discussed at the same time:
I think these are all independent issues, so instead of discussing them in this PR, we should use issues instead and then consolidate our findings in this PR instead of "abusing" this PR as a platform to discuss all open questions surrounding the rewrite. Do you guys agree with the list of different discussions above? |
Good initiative. In essence, yes. But I don't think that those discussions are prerequisites on moving forward with the monorepo approach. The important thing now is figuring out a way to move forward and a clear upgrade path for consumers. The number one breaking change is that apps can rely on a single dependency and import from that. The closest we can get now is: To handle the discussions you mentioned separately from this gives us only one clear way forward: we keep For the next major version we can rethink the internal structure and what to name packages and where to divide them. That would be good because we are currently on the same path as what happened with version 4.0.0 where we had too many changes we wanted to do. |
|
Proposed scope for 5.0.0:
(Note: we can and should decompose new core components into "base" counterparts, even if we do not expose them.) |
@varl that's not the purpose of this PR though. I'd like to collect information about what's going on, what's planned and what's still open for discussion. And instead of discussing stuff here, I'd like to move that to issues and then just reference them in the docs, so whenever anyone wants to know what our plans are or if something is still open for discussion, he/she should just have to look at the docs |
I really like the simplicity of this. And agree we should split base components out whenever possible - the nice thing about this monorepo is that we could expose i.e. This also splits nicely along dependency lines (at least regarding
I agree with @Mohammer5 on this, documenting the conceptual overview and roadmap in the repo itself is awesome 🥇 - I don't think this PR blocks the migration to monorepo if we keep Splitting into issues for those discussions and moving the results of separate topics into this PR (or opening separate PRs and narrowing the scope of this one to just the monorepo migration) sounds like a good plan to me. If anyone DOESN'T think the migration to a monorepo and release of 5.0 is clear and unblocked, 👎thumbs-down this message and we can open a separate discussion. |
Thanks for the reminder! I keep forgetting this is a PR and not an issue, sorry about that. |
After discussion in #21 about what our category names should be, we decided to postpone that discussion and keep it simpler for ui@5 and use core and widgets for now.
After discussion in #21 about what our category names should be, we decided to postpone that discussion and keep it simpler for ui@5 and use core and widgets for now.
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
HendrikThePendric
left a comment
There was a problem hiding this comment.
I think it's fine to merge this in as it is now. I can see we would want to tweak this text quite a bit in the future though. The problem with this text as I see it is:
- There have been long discussions about:
- Separate repos VS a mono-repo
- Which component goes where
- We have then tried to summarise these discussions into a Conceptual design document
- But in actual fact, it is mostly a summary of these two discussions and their outcomes. In the long run our conceptual design docs should probably:
- Take a more holistic approach to why the library is the way it is, and not focus so much the two topics that were discussed in great depth.
- Remove the sections that keep referring back to when this wasn't a mono-repo yet. In time, the mono-repo will be the status quo and reading a justification of why it came about isn't really that helpful for developers who want to get started with
@dhis2/ui.
Having said that, what is written is all completely correct and definitely useful info. So I'm approving and am assuming that we will revisit this document fairly regularly when we revise the documentation.
|
🚀 fire away |
|
🎉 This PR is included in version 5.0.0-alpha.18 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 5.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Eirik <eirik.haugstulen@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Eirik <eirik.haugstulen@gmail.com>
Closes #20
The current state of the
alphaversion lacks proper documentation about structure and motivation.I've collected several opinions, comments & discussions from github and slack and consolidated them into a single document.
The document is should reflect mostly @varl's perspective right now, I haven't tried to change anything yet and prefer that we talk about some of the strategies and concepts in this PR so we can align our thoughts about what we want this monorepo to be (and not to be!).
Here are the github sources I picked most of the initial contents from: