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

Setting headers option for Delimited_Format.Delimited doesn't render correctly #9002

Closed
Tracked by #9007
AdRiley opened this issue Feb 8, 2024 · 5 comments · Fixed by #9146
Closed
Tracked by #9007

Setting headers option for Delimited_Format.Delimited doesn't render correctly #9002

AdRiley opened this issue Feb 8, 2024 · 5 comments · Fixed by #9146
Assignees
Labels
--bug Type: bug -gui d-intermediate Difficulty: some prior knowledge required p-high Should be completed in the next sprint s-info-needed Status: more information needed from submitter x-on-hold
Milestone

Comments

@AdRiley
Copy link
Member

AdRiley commented Feb 8, 2024

Use Case:

I have a csv file that has a header row that the auto-detect mechanism doesn't detect. So I switch to Delimited_Format.Delimited so I can manually set the headers value.

I click headers

image

So far so good. And choose True.

image

I get the result I want in the data, but there are now two issues:

  1. header= is missing
    image
  2. There is no way to switch the option back to Infer. I can toggle it between True and False, but not to Infer.
@AdRiley AdRiley added p-high Should be completed in the next sprint -gui labels Feb 8, 2024
@github-project-automation github-project-automation bot moved this to ❓New in Issues Board Feb 8, 2024
@AdRiley AdRiley added this to the Beta Release milestone Feb 8, 2024
@AdRiley AdRiley added the --bug Type: bug label Feb 8, 2024
@farmaazon farmaazon self-assigned this Feb 12, 2024
@farmaazon
Copy link
Contributor

farmaazon commented Feb 12, 2024

After today @vitvakatu's fixes, you see the arrow down which allows you to pick Infer...

...however, the actual design does not expect clicking at arrows. Boolean widget does not have "editing" state. Perhaps we should just forbid somehow checkbox inside drop downs?

header is still gone.

@farmaazon farmaazon moved this from ❓New to 📤 Backlog in Issues Board Feb 12, 2024
@farmaazon farmaazon added the d-intermediate Difficulty: some prior knowledge required label Feb 12, 2024
@AdRiley
Copy link
Member Author

AdRiley commented Feb 12, 2024

How about rendering as a checkbox, but when you click it opens the dropdown menu again?

@farmaazon
Copy link
Contributor

Ok, header caption is hidden by default - because we display argument name only when it's not set or when it's a top-level argument. Here, the headers is not (top-level arguments are those of Data.read), and once we set value, well, the value is set.

I'm not sure how to proceed @jdunkerley @wdanilo . This design is taken from the old GUI (but there we don't display checkbox widgets). I could:

  1. display not the argument name in placeholders (without value) with widgets if they would not with value set (to keep consistency)
  2. always display argument name in checkbox widget
  3. always display argument name, also for non-toplevel ones with value set.

@farmaazon farmaazon added s-info-needed Status: more information needed from submitter x-on-hold labels Feb 13, 2024
@jdunkerley
Copy link
Member

You are correct old IDE didn't show once we had chosen a value.

If it wasn't the first argument we did get "labels" in the old one:
image

Especially on Boolean values we need it (e.g. skip rows just becomes a tick box).

Up to @wdanilo for design but I think a label of some form is useful

@farmaazon
Copy link
Contributor

farmaazon commented Feb 19, 2024

For this task scope, I will just always display header in boolean widget (expecting that other widgets may need label as well)

@farmaazon farmaazon assigned kazcw and unassigned farmaazon Feb 22, 2024
@kazcw kazcw moved this from 📤 Backlog to ⚙️ Design in Issues Board Feb 22, 2024
@kazcw kazcw moved this from ⚙️ Design to 👁️ Code review in Issues Board Feb 22, 2024
@enso-bot enso-bot bot mentioned this issue Feb 22, 2024
5 tasks
@mergify mergify bot closed this as completed in #9146 Feb 23, 2024
mergify bot pushed a commit that referenced this issue Feb 23, 2024
Fixes #9002. This is not a great solution; doing this properly would require some refactoring of widget selection. Proposal:  #9145.
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Feb 23, 2024
@farmaazon farmaazon moved this from 🟢 Accepted to 🗄️ Archived in Issues Board Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -gui d-intermediate Difficulty: some prior knowledge required p-high Should be completed in the next sprint s-info-needed Status: more information needed from submitter x-on-hold
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants