Show error pop-up when failing to rename a project #6366

Procrat · 2023-04-20T11:58:09Z

Pull Request Description

Closes #5065: when a project can't be renamed, it now shows an error pop-up and stays in edit mode.

space3.mp4

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the Scala, Java, and Rust style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- ~~Unit tests have been written where possible.~~
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

wdanilo

Instead of reverting to an old name, the error should appear and the name should not be accepted, leaving the text field in the edit mode. If someone tried renaming the project to "really_long_and_complex name" and made a typo, reverting the name without giving the user the ability to fix the typo is a really bad UX.

app/gui/src/model/project/synchronized.rs

farmaazon · 2023-04-20T14:38:40Z

Instead of reverting to an old name, the error should appear and the name should not be accepted, leaving the text field in the edit mode. If someone tried renaming the project to "really_long_and_complex name" and made a typo, reverting the name without giving the user the ability to fix the typo is a really bad UX.

@Procrat So, this would need checking it on our side (We cannot keep edit mode waiting for the engine's response).

As the project's name must be a valid identifier, we could just check if it's parsed as Cons (Var also could be accepted, but I'm not sure).

Reverting name when engine returned error should be still kept.

app/gui/view/src/popup.rs

farmaazon · 2023-04-24T09:37:20Z

@Procrat please update the PR description and the video with the new behavior.

Procrat · 2023-04-24T14:23:04Z

@Procrat please update the PR description and the video with the new behavior.

Done.

CHANGELOG.md

app/gui/src/model/project/synchronized.rs

app/gui/view/graph-editor/src/component/breadcrumbs/project_name.rs

app/gui/view/src/popup.rs

* develop: Turn null into UnexpectedExpression when Union type is incomplete (#6415) Ensure that IO.println does not fail if `to_text` returned a non-Text value. (#6223) Improve inlining of `<|` on (GraalVM EE) (#6384) Feat: update templates styles and feature (#6071) 5127 Add Table.parse_to_columns to parse a single column to a set of columns. (#6383) URL handling (#6243) Exclude comparison operators from modifier logic (#6370) Better cleanup of project management test suite (#6392) Fix all eslint errors (#6267) Infer SQLite types locally (#6381) Vector Editor with List Editor and adding elements. (#6363) Add typechecks to Aggregate and Cross Tab (#6380) Forbid edits in read-only mode (#6342) Add Table.parse_text_to_table to convert Text to a Table. (#6294) Adding typechecks to Column Operations (#6298) Rollback cloud options groups (#6331) Project folder not renamed after project name change (#6369) Add `replace`, `trim` to Column. Better number parsing. (#6253) Read-only mode for controllers (#6259) Prevent default for all events, fixing multiple IDE bugs (#6364)

vitvakatu · 2023-04-27T17:43:52Z

QA Report 🔴

I assume the popup with the error should always be visible, even if the scene is panned. This is not the case.

Procrat · 2023-04-28T12:32:09Z

Good catch, @vitvakatu. I've found the reason: I had changed the pop-up's layer to the default layer of Label, which is the tooltip, but that one moves along when panning. Now, the reason why I originally deleted the code that explicitly sets it to the panel layer, is because that created this super weird bug where the other text on that layer just disappears (status bar, project name, breadcrumbs, project switcher, ...). I had spent some time looking into what could cause it, but I'm all out of ideas really.

vitvakatu · 2023-04-28T12:54:53Z

@kazcw @wdanilo do you have any ideas about what can cause this issue?

Procrat · 2023-04-28T13:23:01Z

You can try it out on this branch: wip/procrat/temp-rename-project-with-space-5065-panel-layer

kazcw · 2023-04-28T14:17:59Z

This is in the log:

[WARN] lib/rust/ensogl/component/text/src/component/text.rs:1409 Internal error. Line metrics was not computed.

It might be relevant, as I've never seen it come up before

Edit: The bug always happens, this warning sometimes does not. Probably not related.

kazcw · 2023-04-28T15:34:26Z

It seems related to using multiple fonts in the same layer. If I remove label.set_font(font) from Label, the problem does not occur

kazcw · 2023-04-28T16:33:29Z

I have found the problem. I'll have a fix ready later today.

…-project-with-space-5065

Procrat · 2023-05-02T08:46:22Z

It seems to work with the changes in #6477, so once that's merged, this PR should be good for another round of QA.

Support rendering multiple flavors of a shape system in the same layer. Fixes bugs seen as text disappearing when multiple fonts are used together (#6460 and issue discussed in #6366).

sylwiabr · 2023-05-05T07:56:27Z

@Procrat who is doing a QA here? can we merge it?

Procrat · 2023-05-05T08:19:52Z

@Procrat who is doing a QA here? can we merge it?

@sylwiabr Ilya was going to do another round, but he's been delayed as you know. We can ask someone else if it's urgent.

sylwiabr · 2023-05-05T08:28:03Z

I think Ilia is quite overloaded and will be later today, @mwu-tow or @MichaelMauderer can you do QA here?

MichaelMauderer · 2023-05-05T09:56:13Z

I can have a look today.

vitvakatu

Sorry for the delay, QA passed. Next time please merge develop into this branch, it didn't work without fixes from there. I made a merge myself, please check for conflicts before merging this PR.

* develop: (28 commits) Add tests for Date.until, Date.next and Date.previous. (#6606) Improve `Non_Unique_Primary_Key` error, split file format detection into read/write, improve SQLite format detection (#6604) tokenize_to_columns or parse_to_columns results in a single column we shouldn't add the 1 (#6607) Fix node editing race condition (#6594) Add format to the in-memory Column (#6538) Fix dashboard issues (part 2) (#6511) Fix visualisation type selector artifacts rendered after node preview visualisation was closed. (#6575) Revert typescript CI Lint changes (#6602) Fix the Engine version check in GUI (#6570) Show error pop-up when failing to rename a project (#6366) Small changes from Book Club issues (#6533) "at_least_one" flag for tokenize_to_rows (#6539) Benchmark Engine job runs only engine, not Enso benchmarks (#6534) Catch 5813 and avoid crash (#6585) Fix opening links in desktop IDE (#6507) Identify SyntaxError exception and avoid printing a stack trace (#6574) Fix dashboard issues (#6502) Let ChangesetBuilder.invalidated search even container elements (#6548) Fix #5075: stop panning on full-screen visualisation (#6530) Only `Join_Kind.Inner` removes the common-named columns (#6564) ...

Procrat added 3 commits April 20, 2023 14:07

Extract generic pop-up component from debug mode pop-up

a04b7a2

Show error pop-up when failing to rename a project

cb6186a

Delete obsolete code

cd96702

Procrat force-pushed the wip/procrat/rename-project-with-space-5065 branch from 4fed06f to cd96702 Compare April 20, 2023 12:22

Procrat marked this pull request as ready for review April 20, 2023 12:26

Procrat requested review from MichaelMauderer, wdanilo, farmaazon, kazcw and mwu-tow as code owners April 20, 2023 12:26

wdanilo requested changes Apr 20, 2023

View reviewed changes

kazcw approved these changes Apr 20, 2023

View reviewed changes

app/gui/src/model/project/synchronized.rs Outdated Show resolved Hide resolved

app/gui/src/model/project/synchronized.rs Outdated Show resolved Hide resolved

farmaazon requested changes Apr 20, 2023

View reviewed changes

app/gui/view/src/popup.rs Outdated Show resolved Hide resolved

app/gui/view/src/popup.rs Outdated Show resolved Hide resolved

Procrat added 2 commits April 21, 2023 11:44

Fix PR comments

bb04379

Leave text field in edit mode when name is invalid

b5c1432

Procrat requested review from wdanilo and farmaazon April 24, 2023 08:51

farmaazon approved these changes Apr 24, 2023

View reviewed changes

Procrat requested review from wdanilo and removed request for wdanilo April 24, 2023 14:23

wdanilo approved these changes Apr 25, 2023

View reviewed changes

Procrat added 2 commits April 25, 2023 17:23

Fixups

5b6ffe8

vitvakatu mentioned this pull request Apr 28, 2023

Entering debug mode disappears text rendered in top bar and code window #6460

Closed

2 tasks

kazcw mentioned this pull request Apr 29, 2023

Multiflavor layers #6477

Merged

5 tasks

farmaazon self-assigned this May 1, 2023

farmaazon and others added 2 commits May 1, 2023 10:49

Merge remote-tracking branch 'origin/develop' into wip/procrat/rename…

2e6c62e

…-project-with-space-5065

Put pop-up back to panel layer

8a31694

mergify bot pushed a commit that referenced this pull request May 2, 2023

Multiflavor layers (#6477)

a15086f

Support rendering multiple flavors of a shape system in the same layer. Fixes bugs seen as text disappearing when multiple fonts are used together (#6460 and issue discussed in #6366).

Merge branch 'develop' into wip/procrat/rename-project-with-space-5065

629e951

vitvakatu approved these changes May 5, 2023

View reviewed changes

Procrat added CI: Ready to merge This PR is eligible for automatic merge CI: Keep up to date Automatically update this PR to the latest develop. labels May 5, 2023

mergify bot and others added 7 commits May 5, 2023 13:30

Merge branch 'develop' into wip/procrat/rename-project-with-space-5065

6fbcf49

Merge branch 'develop' into wip/procrat/rename-project-with-space-5065

77af171

Merge branch 'develop' into wip/procrat/rename-project-with-space-5065

8db3b86

Merge branch 'develop' into wip/procrat/rename-project-with-space-5065

d0dfb3f

Merge branch 'develop' into wip/procrat/rename-project-with-space-5065

9ff23e3

Merge branch 'develop' into wip/procrat/rename-project-with-space-5065

96995dc

Bump Wasm size limit

523c6e1

mergify bot merged commit 069fcf3 into develop May 8, 2023

mergify bot deleted the wip/procrat/rename-project-with-space-5065 branch May 8, 2023 10:12

Procrat mentioned this pull request May 11, 2023

Project folder renaming stopped working #6515

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show error pop-up when failing to rename a project #6366

Show error pop-up when failing to rename a project #6366

Procrat commented Apr 20, 2023 •

edited

Loading

wdanilo left a comment

farmaazon commented Apr 20, 2023 •

edited

Loading

farmaazon commented Apr 24, 2023

Procrat commented Apr 24, 2023

vitvakatu commented Apr 27, 2023

Procrat commented Apr 28, 2023

vitvakatu commented Apr 28, 2023

Procrat commented Apr 28, 2023

kazcw commented Apr 28, 2023 •

edited

Loading

kazcw commented Apr 28, 2023

kazcw commented Apr 28, 2023

Procrat commented May 2, 2023

sylwiabr commented May 5, 2023

Procrat commented May 5, 2023

sylwiabr commented May 5, 2023

MichaelMauderer commented May 5, 2023

vitvakatu left a comment

Show error pop-up when failing to rename a project #6366

Show error pop-up when failing to rename a project #6366

Conversation

Procrat commented Apr 20, 2023 • edited Loading

Pull Request Description

Checklist

wdanilo left a comment

Choose a reason for hiding this comment

farmaazon commented Apr 20, 2023 • edited Loading

farmaazon commented Apr 24, 2023

Procrat commented Apr 24, 2023

vitvakatu commented Apr 27, 2023

QA Report 🔴

Procrat commented Apr 28, 2023

vitvakatu commented Apr 28, 2023

Procrat commented Apr 28, 2023

kazcw commented Apr 28, 2023 • edited Loading

kazcw commented Apr 28, 2023

kazcw commented Apr 28, 2023

Procrat commented May 2, 2023

sylwiabr commented May 5, 2023

Procrat commented May 5, 2023

sylwiabr commented May 5, 2023

MichaelMauderer commented May 5, 2023

vitvakatu left a comment

Choose a reason for hiding this comment

Procrat commented Apr 20, 2023 •

edited

Loading

farmaazon commented Apr 20, 2023 •

edited

Loading

kazcw commented Apr 28, 2023 •

edited

Loading