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: listbox properties spec #1228

Merged
merged 3 commits into from
May 2, 2023
Merged

feat: listbox properties spec #1228

merged 3 commits into from
May 2, 2023

Conversation

T-Wizard
Copy link
Collaborator

@T-Wizard T-Wizard commented Apr 19, 2023

Start of ListboxProperties spec

Motivation

Requirements checklist

  • Api specification
    • Ran yarn spec
      • No changes OR API changes has been formally approved
  • Unit/Component test coverage
  • Correct PR title for the changes (fix, chore, feat)

When build and tests have passed:

  • Add code reviewers, for example @qlik-oss/nebula-core

@T-Wizard T-Wizard force-pushed the listbox-properties-spec branch 2 times, most recently from 77da6af to ad17948 Compare April 24, 2023 09:31
@T-Wizard T-Wizard marked this pull request as ready for review April 24, 2023 11:21
@Caele
Copy link
Collaborator

Caele commented Apr 25, 2023

Should we really put this here? Think it will just be more confusing if we mix this into the other Nebula stuff. Could instead reference a different api, either a nebula-list-box or the sn-listbox one.

@veinfors
Copy link
Collaborator

Should we really put this here? Think it will just be more confusing if we mix this into the other Nebula stuff. Could instead reference a different api, either a nebula-list-box or the sn-listbox one.

Sure, can agree that it could be nice to have it as a separate Api. We have already discussed it a little bit back and forth, but let's have a little talk about it tomorrow and decide what to do.

@T-Wizard
Copy link
Collaborator Author

T-Wizard commented Apr 26, 2023

Should we really put this here? Think it will just be more confusing if we mix this into the other Nebula stuff. Could instead reference a different api, either a nebula-list-box or the sn-listbox one.

If we split it to a different api, I don't know how to reference it in a way that also get the types for the generated typescript definition.

@Caele: except that it it something like #1245 you want instead?
(if not I need more info about what you want)

@T-Wizard T-Wizard mentioned this pull request Apr 28, 2023
6 tasks
Copy link
Collaborator

@Caele Caele left a comment

Choose a reason for hiding this comment

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

LGTM
Don't really know how we "solve" typings for TS, but I think this is a safer approach to get us started.

@veinfors veinfors added the 4.0.0 label Apr 28, 2023
@T-Wizard T-Wizard merged commit 60a977f into master May 2, 2023
@T-Wizard T-Wizard deleted the listbox-properties-spec branch May 2, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants